Skip to content

Commit c66b428

Browse files
vaussardcfriedt
authored andcommitted
net: ipv6_fragment: fix shift_packets() algorithm
The purpose of shift_packets() is to make room to insert one fragment in the list. This is not what it does currently, potentially leading to -ENOMEM even if there is enough free room. To see the current behaviour, let's assume that we receive 3 fragments in reverse order: - Frag3(offset = 0x40, M=0) - Frag2(offset = 0x20, M=1) - Frag1(offset = 0x00, M=1) After receiving Frag3 and Frag2, pkt[] will look like: .-------.-------.-------. | Frag2 | Frag3 | NULL | | 0x20 | 0x40 | | '-------'-------'-------' pkt[0] pkt[1] pkt[2] When receiving Frag1, shift_packets(pos = 0) is called to make some room at position 0. It will iterate up to i = 2 where there is a free element. The current algorithm will try to shift pkt[0] to pkt[2], which is indeed impossible but also unnecessary. It is only required to shift pkt[0] and pkt[1] by one element in order to free pkt[0] to insert Frag1. Update the algorithm in order to shift the memory only by one element. As a result, the ENOMEM test is only simpler: as long as we encounter one free element, we are guaranteed that we can shift by one element. Also assign a NULL value to the newly freed element since memmove() only copy bytes. Signed-off-by: Florian Vaussard <[email protected]>
1 parent a9917d9 commit c66b428

File tree

1 file changed

+9
-10
lines changed

1 file changed

+9
-10
lines changed

subsys/net/ip/ipv6_fragment.c

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -432,24 +432,23 @@ static int shift_packets(struct net_ipv6_reassembly *reass, int pos)
432432
NET_DBG("Moving [%d] %p (offset 0x%x) to [%d]",
433433
pos, reass->pkt[pos],
434434
net_pkt_ipv6_fragment_offset(reass->pkt[pos]),
435-
i);
435+
pos + 1);
436436

437-
/* Do we have enough space in packet array to make
438-
* the move?
437+
/* pkt[i] is free, so shift everything between
438+
* [pos] and [i - 1] by one element
439439
*/
440-
if (((i - pos) + 1) >
441-
(NET_IPV6_FRAGMENTS_MAX_PKT - i)) {
442-
return -ENOMEM;
443-
}
444-
445-
memmove(&reass->pkt[i], &reass->pkt[pos],
440+
memmove(&reass->pkt[pos + 1], &reass->pkt[pos],
446441
sizeof(void *) * (i - pos));
447442

443+
/* pkt[pos] is now free */
444+
reass->pkt[pos] = NULL;
445+
448446
return 0;
449447
}
450448
}
451449

452-
return -EINVAL;
450+
/* We do not have free space left in the array */
451+
return -ENOMEM;
453452
}
454453

455454
enum net_verdict net_ipv6_handle_fragment_hdr(struct net_pkt *pkt,

0 commit comments

Comments
 (0)