Stefan Schmidt <stefan@xxxxxxxxxxxxxxxxxx> wrote:
> "We have tested the 6LoWPAN modules in the Linux kernel and came to some
> issue regarding fragmentation. We have seen aborted SCP transfers
> ("message authentication code incorrect") and tested TCP transfers as
> well and saw corruption on fragment-sized intervals. The current
> fragment assembling functions do not check enough for corrupted L2
> packets that might slip through L2 CRC check. (in our case IEEE802.15.4
> which has only 16-bit CRC).
> As a result, overlapping fragments due to offset corruption are not
> detected and assembled incorrectly. Part of packets may have old data.
> At TCP-level, there is only a simple TCP-checksum which is not enough in
> this case as the corruption occurs frequently (once every few minutes).
This is a common problem with checksums, fragments and "high" bandwidths.
It can occur with some frequency, and has been a reason for much of the PMTU
work (including PLMTUD) to get rid of IP-level fragmentation.
A solution is better checksums: or rather integrity hashes.
I'm curious if the problem would have been if the 802.15.4 network was
encrypted, and thus had an cryptographic integrity check.
> After quickly analysing the code we saw some potential issues and
> created a patch that adds additional overlap checks and simplifies some
> conditional statements. After running tests again, TCP corruption was
> not seen again. The test was performed with SCP and keeps transferring
> large files now without error."
> It would be great if some of this finds it way into the commit log of
> these two patches. At a minimum I would require them to not have the
> same commit summary line.
Agreed.
>> Signed-off-by: Rafael Vuijk <r.vuijk@xxxxxxxxx>
>> --- ./net/ieee802154/6lowpan/reassembly.c 2018-02-20 11:10:06.000000000 +0100
>> +++ ./net/ieee802154/6lowpan/reassembly.c 2018-02-21 09:13:29.000000000 +0100
>> @@ -140,23 +140,14 @@ static int lowpan_frag_queue(struct lowp
>> offset = lowpan_802154_cb(skb)->d_offset << 3;
>> end = lowpan_802154_cb(skb)->d_size;
>>
>> + if (fq->q.len == 0)
>> + fq->q.len = end;
>> + if (fq->q.len != end)
>> + goto err;
>> +
>> /* Is this the final fragment? */
>> if (offset + skb->len == end) {
>> - /* If we already have some bits beyond end
>> - * or have different end, the segment is corrupted.
>> - */
>> - if (end < fq->q.len ||
>> - ((fq->q.flags & INET_FRAG_LAST_IN) && end != fq->q.len))
>> - goto err;
fq-> q.flags |= INET_FRAG_LAST_IN;
>> - fq->q.len = end;
>> - } else {
>> - if (end > fq->q.len) {
>> - /* Some bits beyond end -> corruption. */
>> - if (fq->q.flags & INET_FRAG_LAST_IN)
>> - goto err;
>> - fq->q.len = end;
>> - }
> Some basic testing on my side has not revealed any issues on this. I
> will give it some longer testing with SCP now.
> regards
> Stefan Schmidt
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wpan" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
