crcsync byte-at-a-time test reveals flaws in buffer handling.

(Reported by Alex Wulms <alex.wulms@scarlet.be>)

diff --git a/ccan/crcsync/crcsync.c b/ccan/crcsync/crcsync.c
index 6c251f1ff967d1c1a49a9a42592f50926e1223c0..ccc648a51bf4b4389bd4be8083542c5dafb35b7c 100644 (file)
--- 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;
        }
 
                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
        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++;
                                                    *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 {
                        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++;
        }
 
                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) {
        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;
                        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;
                }
        } 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;
 
                } 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;
 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;
 }
 
        return ret;
 }
 

diff --git a/ccan/crcsync/test/api.c b/ccan/crcsync/test/api.c
index ed17f8e6050e78b6bdb88e7e5a15282dbe5b57db..2f07cb696e1fe2ea10516ae9b163ca2aa5406c0e 100644 (file)
--- 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;
                      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);
        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));
 
        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);
        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);
        }
 
                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];
 
        unsigned int i;
        uint32_t crcs1[12], crcs2[12];
 

-       plan_tests(733);
+       plan_tests(1454);

 
        buffer1 = calloc(1024, 1);
        buffer2 = calloc(1024, 1);
 
        buffer1 = calloc(1024, 1);
        buffer2 = calloc(1024, 1);