Re: [PATCH v2 net-next] tcp: Warn if sock_owned_by_user() is true in tcp_child_process().

[Kuniyuki Iwashima <kuniyu@amazon.co.jp>]

From:   Paolo Abeni <pabeni@redhat.com>
Date:   Thu, 09 Dec 2021 12:59:21 +0100
> On Thu, 2021-12-09 at 20:07 +0900, Kuniyuki Iwashima wrote:
>> From:   Eric Dumazet <edumazet@google.com>
>> Date:   Thu, 9 Dec 2021 00:00:35 -0800
>>> On Wed, Dec 8, 2021 at 5:33 PM Kuniyuki Iwashima <kuniyu@amazon.co.jp> wrote:
>>>> 
>>>> While creating a child socket from ACK (not TCP Fast Open case), before
>>>> v2.3.41, we used to call bh_lock_sock() later than now; it was called just
>>>> before tcp_rcv_state_process().  The full socket was put into an accept
>>>> queue and exposed to other CPUs before bh_lock_sock() so that process
>>>> context might have acquired the lock by then.  Thus, we had to check if any
>>>> process context was accessing the socket before tcp_rcv_state_process().
>>>> 
>>> 
>>> I think you misunderstood me.
>>> 
>>> I think this code is not dead yet, so I would :
>>> 
>>> Not include a Fixes: tag to avoid unnecessary backports (of a patch
>>> and its revert)
>>> 
>>> If you want to get syzbot coverage for few releases, especially with
>>> MPTCP and synflood,
>>> you  can then submit a patch like the following.
>> 
>> Sorry, I got on the same page.
>> Let me take a look at MPTCP, then if I still think it is dead code, I will
>> submit the patch.
> 
> For the records, I think the 'else' branch should be reachble with
> MPTCP in some non trivial scenario, e.g. MPJ subflows 3WHS racing with
> setsockopt on the main MPTCP socket. I'm unsure if syzbot could catch
> that, as it needs mptcp endpoints configuration.

Ah, I was wrong.
Thanks for explaining!


> 
> Cheers,
> 
> Paolo