From 686061a501169c08d35708010f1c7c7b0eb13d77 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 30 Mar 2009 10:38:38 +1030 Subject: [PATCH] crcsync byte-at-a-time test reveals flaws in buffer handling. (Reported by Alex Wulms ) --- ccan/crcsync/crcsync.c | 55 ++++++++++++++++++++++------------------- ccan/crcsync/test/api.c | 25 +++++++++++++++++-- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/ccan/crcsync/crcsync.c b/ccan/crcsync/crcsync.c index 6c251f1f..ccc648a5 100644 --- a/ccan/crcsync/crcsync.c +++ b/ccan/crcsync/crcsync.c @@ -138,6 +138,7 @@ size_t crc_read_block(struct crc_context *ctx, long *result, goto have_match; } + /* old is the trailing edge of the checksum window. */ if (buffer_size(ctx) >= ctx->block_size) old = ctx->buffer + ctx->buffer_start; else @@ -153,6 +154,7 @@ size_t crc_read_block(struct crc_context *ctx, long *result, *old, *p, ctx->uncrc_tab); old++; + /* End of stored buffer? Start on data they gave us. */ if (old == ctx->buffer + ctx->buffer_end) old = buf; } else { @@ -167,11 +169,6 @@ size_t crc_read_block(struct crc_context *ctx, long *result, p++; } - /* Make sure we have a copy of the last block_size bytes. - * First, copy down the old data. */ - if (buffer_size(ctx)) { - } - if (crcmatch >= 0) { /* We have a match! */ if (ctx->literal_bytes > ctx->block_size) { @@ -187,12 +184,15 @@ size_t crc_read_block(struct crc_context *ctx, long *result, assert(ctx->literal_bytes == 0); ctx->have_match = -1; ctx->running_crc = 0; + /* Nothing more in the buffer. */ + ctx->buffer_start = ctx->buffer_end = 0; } } else { /* Output literal if it's more than 1 block ago. */ if (ctx->literal_bytes > ctx->block_size) { *result = ctx->literal_bytes - ctx->block_size; - ctx->literal_bytes = ctx->block_size; + ctx->literal_bytes -= *result; + ctx->buffer_start += *result; } else *result = 0; @@ -243,29 +243,34 @@ static size_t final_block_match(struct crc_context *ctx) long crc_read_flush(struct crc_context *ctx) { long ret; + size_t final; - /* In case we ended on a whole block match. */ - if (ctx->have_match == -1) { - size_t final; - - final = final_block_match(ctx); - if (!final) { - /* This is how many bytes we're about to consume. */ - ret = buffer_size(ctx); - ctx->buffer_start += ret; - ctx->literal_bytes -= ret; + /* We might have ended right on a matched block. */ + if (ctx->have_match != -1) { + ctx->literal_bytes -= ctx->block_size; + assert(ctx->literal_bytes == 0); + ret = -ctx->have_match-1; + ctx->have_match = -1; + ctx->running_crc = 0; + /* Nothing more in the buffer. */ + ctx->buffer_start = ctx->buffer_end; + return ret; + } - return ret; - } - ctx->buffer_start += final; - ctx->literal_bytes -= final; - ctx->have_match = ctx->num_crcs-1; + /* Look for truncated final block. */ + final = final_block_match(ctx); + if (!final) { + /* Nope? Just a literal. */ + ret = buffer_size(ctx); + ctx->buffer_start += ret; + ctx->literal_bytes -= ret; + return ret; } - /* It might be a partial block match, so no assert */ - ctx->literal_bytes = 0; - ret = -ctx->have_match-1; - ctx->have_match = -1; + /* We matched (some of) what's left. */ + ret = -(ctx->num_crcs-1)-1; + ctx->buffer_start += final; + ctx->literal_bytes -= final; return ret; } diff --git a/ccan/crcsync/test/api.c b/ccan/crcsync/test/api.c index ed17f8e6..2f07cb69 100644 --- a/ccan/crcsync/test/api.c +++ b/ccan/crcsync/test/api.c @@ -64,15 +64,36 @@ static void test_sync(const char *buffer1, size_t len1, const struct result results[], size_t num_results) { struct crc_context *ctx; - size_t used, ret, i, curr_literal = 0; + size_t used, ret, i, curr_literal; long result; uint32_t crcs[num_blocks(len1, block_size)]; crc_of_blocks(buffer1, len1, block_size, 32, crcs); + + /* Normal method. */ ctx = crc_context_new(block_size, 32, crcs, ARRAY_SIZE(crcs)); + curr_literal = 0; for (used = 0, i = 0; used < len2; used += ret) { ret = crc_read_block(ctx, &result, buffer2+used, len2-used); + check_result(result, &curr_literal, results, num_results, &i); + } + + while ((result = crc_read_flush(ctx)) != 0) + check_result(result, &curr_literal, results, num_results, &i); + + check_finalized_result(curr_literal, results, num_results, &i); + + /* We must have achieved everything we expected. */ + ok1(i == num_results); + crc_context_free(ctx); + + /* Byte-at-a-time method. */ + ctx = crc_context_new(block_size, 32, crcs, ARRAY_SIZE(crcs)); + + curr_literal = 0; + for (used = 0, i = 0; used < len2; used += ret) { + ret = crc_read_block(ctx, &result, buffer2+used, 1); check_result(result, &curr_literal, results, num_results, &i); } @@ -93,7 +114,7 @@ int main(int argc, char *argv[]) unsigned int i; uint32_t crcs1[12], crcs2[12]; - plan_tests(733); + plan_tests(1454); buffer1 = calloc(1024, 1); buffer2 = calloc(1024, 1); -- 2.39.2