netdev - Re: [PATCH net-next v2 1/2] tcp: Remove unnecessary dst check in tcp_conn

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] [day] [month] [year] [list]

Date:   Tue, 22 Aug 2017 08:56:00 +0800
From:   Tonghao Zhang <xiangxia.m.yue@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     David Miller <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next v2 1/2] tcp: Remove unnecessary dst check in tcp_conn_request.

On Mon, Aug 21, 2017 at 10:56 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> Please do not top post.
Got it, thanks.

> On Mon, 2017-08-21 at 21:24 +0800, Tonghao Zhang wrote:
>> Thanks,  yes  this is a bug. I found this bug exists from 3.17~ 4.13.
>> The commit is d94e0417
>>
>
> This bug was there at the beginning of git tree.
>
>
>> One question:  should I send a patch for each kernel version because
>> code conflicts ?
>>
>> a patch for v4.12
>> a patch for v4.11
>> a patch for v4.10~v4.7
>> a patch for v4.6~v3.17
>>
>> and
>> a patch for  net-next, because tcp_tw_recycle has been removed.
>>
>
> Given this bug only would matter if syncookies are disabled, I would not
> bother and only target net-next. This does not look serious enough to
> deserve backports to stable versions.
>
OK, thanks again.

>> Thanks very much.
>>
>> On Sun, Aug 20, 2017 at 12:25 PM, David Miller <davem@...emloft.net> wrote:
>> > From: Tonghao Zhang <xiangxia.m.yue@...il.com>
>> > Date: Wed, 16 Aug 2017 20:02:45 -0700
>> >
>> >> Because we remove the tcp_tw_recycle support in the commit
>> >> 4396e46187c ('tcp: remove tcp_tw_recycle') and also delete
>> >> the code 'af_ops->route_req' for sysctl_tw_recycle in tcp_conn_request.
>> >> Now when we call the 'af_ops->route_req', the dist always is
>> >> NULL, and we remove the unnecessay check.
>> >>
>> >> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
>> >
>> > This is a bug actually, rather than something to paper over
>> > by removing the check.
>> >
>> > Code earlier in this function needs a proper 'dst' in order to operate
>> > properly.
>> >
>> > There is a call to tcp_peer_is_proven() which must have a proper route
>> > to make the determination yet it will always be NULL.
>> >
>> > Please investigate what the code is doing and how a test became
>> > "unnecessary" over time before blindly removing it, thank you.
>
>