netdev - Re: [PATCH net] net: diag: Don't double-free TCP_NEW_SYN_RECV sockets in tcp

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]

Message-ID: <fd71a365-e25c-070f-bc52-38fc89c7acb4@cumulusnetworks.com>
Date:   Mon, 9 Jul 2018 10:14:23 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        Lorenzo Colitti <lorenzo@...gle.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        Alistair Strachan <astrachan@...gle.com>,
        Subash Abhinov Kasiviswanathan <subashab@...eaurora.org>
Subject: Re: [PATCH net] net: diag: Don't double-free TCP_NEW_SYN_RECV sockets
 in tcp_abort

On 7/9/18 9:17 AM, Eric Dumazet wrote:
> 
> 
> On 07/09/2018 07:59 AM, David Ahern wrote:
>> On 7/8/18 10:53 PM, Lorenzo Colitti wrote:
>>> On Sat, Jul 7, 2018 at 10:29 PM Eric Dumazet <eric.dumazet@...il.com> wrote:
>>>>>> Tested: passes Android sock_diag_test.py, which exercises this codepath
>>>>>
>>>>> If this Android test case exercises this path, why didn't it trigger
>>>>> the double free and thus cause this bug to be found much sooner?
>>>>>
>>>>> Just curious.
>>>>
>>>> Presumably android has not backported yet the refcount_t stuff in their kernels.
>>>
>>> That's correct. We only started seeing this on 4.14, which is not yet
>>> in the field.
>>
>> Prior to 4.12 syn-recv sockets did not show up in the list, so it could
>> not be closed through the diag API. I did not chase it to a specific
>> commit between 4.11 (no syn-recv in 'ss -ta') and 4.12 (syn-recv shows
>> up in 'ss -ta').
>>
> 
> I suggest you look more closely to tcp_abort(), the function you changed
> in commit d7226c7a4dd1  (linux-4.8)
> 
> Then you will see it had support for syn-recv since linux-4.5
> 
> So I do not understand your 4.11/4.12 mentions at all.
> 
> "ss -ta" always had dumped syn-recv request sockets.
> 
> 

Perhaps it is something with my config, settings, ss version or test
program, but I do not see it on 4.11:

# ip addr sh dev eth1
3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc mq state UP
group default qlen 1000
    link/ether 02:e0:f9:1c:00:37 brd ff:ff:ff:ff:ff:ff
    inet 10.100.1.4/24 scope global eth1
       valid_lft forever preferred_lft forever
...

# uname -r
4.11.0

# iptables -A OUTPUT -p tcp --sport 12345  --tcp-flags SYN,ACK SYN,ACK
-j DROP
# vrf-test -s &
[1] 823

--> connection attempt into this node from 10.100.1.254

# ss -ta | grep 1234
LISTEN     0      5          *:12345                    *:*

No, SYN-RECV in output. A local connection does show up (but I was not
looking at that earlier):

# vrf-test -r 10.100.1.4 &
[2] 827

# ss -ta | grep 1234
LISTEN     0      5          *:12345                    *:*
SYN-RECV   0      0      10.100.1.4:12345                10.100.1.254:54276
SYN-RECV   0      0      10.100.1.4:12345                10.100.1.4:52316
SYN-SENT   0      1      10.100.1.4:52316                10.100.1.4:12345


Same commands on 4.12:

# uname -r
4.12.0

# iptables -A OUTPUT -p tcp --sport 12345  --tcp-flags SYN,ACK SYN,ACK
-j DROP
# vrf-test -s &
[1] 815

--> connection attempt into this node from 10.100.1.254

# ss -ta | grep 1234
LISTEN     0      5          *:12345                    *:*
SYN-RECV   0      0      10.100.1.4:12345                10.100.1.254:54272

So, SYN-RECV is there.

However, in writing this response and repeating the steps over and over
I have noticed inconsistencies even with 4.12. For example, I reboot
4.12 and try the steps again:

# uname -r
4.12.0

# iptables -A OUTPUT -p tcp --sport 12345  --tcp-flags SYN,ACK SYN,ACK
-j DROP
# vrf-test -s &
[1] 821

--> connection attempt into this node from 10.100.1.254

# ss -ta | grep 1234
LISTEN     0      5          *:12345                    *:*

no SYN-RECV. Local test:

# vrf-test -r 10.100.1.4 &
[2] 824
# ss -ta | grep 1234
LISTEN     0      5          *:12345                    *:*
SYN-SENT   0      1      10.100.1.4:59922                10.100.1.4:12345

No SYN-RECV.

# ss --version
ss utility, iproute2-ss150831