On Wed, May 4, 2022 at 6:09 AM Paolo Abeni [off-list ref] wrote:

On Tue, 2022-05-03 at 14:17 -0700, Eric Dumazet wrote:

quoted

On Tue, May 3, 2022 at 4:40 AM [off-list ref] wrote:

quoted

Hello:

This patch was applied to netdev/net.git (master)
by Paolo Abeni [off-list ref]:

On Mon, 2 May 2022 10:40:18 +0900 you wrote:

quoted

syzbot is reporting use-after-free read in tcp_retransmit_timer() [1],
for TCP socket used by RDS is accessing sock_net() without acquiring a
refcount on net namespace. Since TCP's retransmission can happen after
a process which created net namespace terminated, we need to explicitly
acquire a refcount.

Link: https://syzkaller.appspot.com/bug?extid=694120e1002c117747ed [1]
Reported-by: syzbot <redacted>
Fixes: 26abe14379f8e2fa ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
Fixes: 8a68173691f03661 ("net: sk_clone_lock() should only do get_net() if the parent is not a kernel socket")
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot <redacted>

[...]

Here is the summary with links:
  - [v2] net: rds: acquire refcount on TCP sockets
    https://git.kernel.org/netdev/net/c/3a58f13a881e

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

I think we merged this patch too soon.

My fault.

quoted

My question is : What prevents rds_tcp_conn_path_connect(), and thus
rds_tcp_tune() to be called
after the netns refcount already reached 0 ?

I guess we can wait for next syzbot report, but I think that get_net()
should be replaced
by maybe_get_net()

Should we revert this patch before the next pull request, if a suitable
incremental fix is not available by then?

It looks like the window of opportunity for the race is roughly the
same?

No need to revert the patch, we certainly are in a better situation,
as refcount_t helps here.

We can refine the logic in a followup.

Thanks.