From 027abc36cfb0a8247625f5f6af65b7ce63065a33 Mon Sep 17 00:00:00 2001 From: Guus Sliepen Date: Mon, 19 Oct 2015 22:04:50 +0200 Subject: [PATCH] Fix bugs in sack_consume() causing data corruption or abort()s. Put in a lengthy comment with some ASCII-art to describe what we are actually trying to do in this function. --- utcp.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/utcp.c b/utcp.c index 7c57701..607af2d 100644 --- a/utcp.c +++ b/utcp.c @@ -633,7 +633,24 @@ cleanup: free(pkt); } -// Update receive buffer and SACK entries after consuming data. +/* Update receive buffer and SACK entries after consuming data. + * + * Situation: + * + * |.....0000..1111111111.....22222......3333| + * |---------------^ + * + * 0..3 represent the SACK entries. The ^ indicates up to which point we want + * to remove data from the receive buffer. The idea is to substract "len" + * from the offset of all the SACK entries, and then remove/cut down entries + * that are shifted to before the start of the receive buffer. + * + * There are three cases: + * - the SACK entry is ahead of ^, in that case just change the offset. + * - the SACK entry starts before and ends after ^, so we have to + * change both its offset and size. + * - the SACK entry is completely before ^, in that case delete it. + */ static void sack_consume(struct utcp_connection *c, size_t len) { debug("sack_consume %zu\n", len); if(len > c->rcvbuf.used) @@ -646,13 +663,13 @@ static void sack_consume(struct utcp_connection *c, size_t len) { c->sacks[i].offset -= len; i++; } else if(len < c->sacks[i].offset + c->sacks[i].len) { - c->sacks[i].offset = 0; c->sacks[i].len -= len - c->sacks[i].offset; + c->sacks[i].offset = 0; i++; } else { if(i < NSACKS - 1) { memmove(&c->sacks[i], &c->sacks[i + 1], (NSACKS - 1 - i) * sizeof c->sacks[i]); - c->sacks[i + 1].len = 0; + c->sacks[NSACKS - 1].len = 0; } else { c->sacks[i].len = 0; break; -- 2.39.5