netdev - Re: [PATCH net-next v13 3/7] sch_cake: Add optional ACK filter

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 21 May 2018 19:35:35 +0200
From:   Toke Høiland-Jørgensen <toke@...e.dk>
To:     Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
        cake@...ts.bufferbloat.net
Subject: Re: [PATCH net-next v13 3/7] sch_cake: Add optional ACK filter

Eric Dumazet <eric.dumazet@...il.com> writes:

> On 05/21/2018 09:24 AM, Toke Høiland-Jørgensen wrote:
>
>> +		while (oplen_tmp >= 8) {
>> +			u32 right_b = get_unaligned_be32(sack_tmp + 4);
>> +			u32 left_b = get_unaligned_be32(sack_tmp);
>> +
>> +			if (left_b >= right_b)
>> +				continue;
>> +
>> +			if (first)
>> +				bytes_b += right_b - left_b;
>> +
>> +			if (left_b <= left_a && right_a <= right_b) {
>> +				found = true;
>> +				if (!first)
>> +					break;
>> +			}
>> +			oplen_tmp -= 8;
>> +			sack_tmp += 8;
>> +		}
>>
>
> This is obviously incorrect.
>
> Please take a look at TCP stack, and how it handles sequence wrapping
> with following macros :
>
> after(), before()

Ah yes, sequence number wrapping. I was thinking I needed to deal with
that, and then got sidetracked and forgot about it. Will fix.

Other than that, do you agree that this approach to SACK and header
handling can work?

> Quite frankly O wonder how this was really tested.

Getting it wrong results in a few drop decisions being wrong every 2**32
bytes; easy to miss. If you have a "mis-behaving middle-box test suite"
or something I'll be happy to hear about it :)

-Toke