[PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Kuniyuki Iwashima]

syzkaller reported a use-after-free of kernel UDP socket in
cleanup_bearer() without repro. [0][1]

When bearer_disable() calls tipc_udp_disable(), cleanup of the kernel
UDP socket is deferred by work calling cleanup_bearer().

Since the cited commit, however, the socket's netns might not be alive
when the work is executed, resulting in use-after-free.

Let's hold netns for the kernel UDP socket when created.

Note that we can't call get_net() before scheduling the work and call
put_net() in cleanup_bearer() because bearer_disable() could be called
from pernet_operations.exit():

  tipc_exit_net
  `- tipc_net_stop
     `- tipc_bearer_stop
        `- bearer_disable

[0]:
ref_tracker: net notrefcnt@000000009b3d1faf has 1/1 users at
     sk_alloc+0x438/0x608
     inet_create+0x4c8/0xcb0
     __sock_create+0x350/0x6b8
     sock_create_kern+0x58/0x78
     udp_sock_create4+0x68/0x398
     udp_sock_create+0x88/0xc8
     tipc_udp_enable+0x5e8/0x848
     __tipc_nl_bearer_enable+0x84c/0xed8
     tipc_nl_bearer_enable+0x38/0x60
     genl_family_rcv_msg_doit+0x170/0x248
     genl_rcv_msg+0x400/0x5b0
     netlink_rcv_skb+0x1dc/0x398
     genl_rcv+0x44/0x68
     netlink_unicast+0x678/0x8b0
     netlink_sendmsg+0x5e4/0x898
     ____sys_sendmsg+0x500/0x830

[1]:
BUG: KMSAN: use-after-free in udp_hashslot include/net/udp.h:85 [inline]
BUG: KMSAN: use-after-free in udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
 udp_hashslot include/net/udp.h:85 [inline]
 udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
 sk_common_release+0xaf/0x3f0 net/core/sock.c:3820
 inet_release+0x1e0/0x260 net/ipv4/af_inet.c:437
 inet6_release+0x6f/0xd0 net/ipv6/af_inet6.c:489
 __sock_release net/socket.c:658 [inline]
 sock_release+0xa0/0x210 net/socket.c:686
 cleanup_bearer+0x42d/0x4c0 net/tipc/udp_media.c:819
 process_one_work kernel/workqueue.c:3229 [inline]
 process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
 worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
 kthread+0x531/0x6b0 kernel/kthread.c:389
 ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244

Uninit was created at:
 slab_free_hook mm/slub.c:2269 [inline]
 slab_free mm/slub.c:4580 [inline]
 kmem_cache_free+0x207/0xc40 mm/slub.c:4682
 net_free net/core/net_namespace.c:454 [inline]
 cleanup_net+0x16f2/0x19d0 net/core/net_namespace.c:647
 process_one_work kernel/workqueue.c:3229 [inline]
 process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
 worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
 kthread+0x531/0x6b0 kernel/kthread.c:389
 ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
 ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244

CPU: 0 UID: 0 PID: 54 Comm: kworker/0:2 Not tainted 6.12.0-rc1-00131-gf66ebf37d69c #7 91723d6f74857f70725e1583cba3cf4adc716cfa
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
Workqueue: events cleanup_bearer

Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
I'll remove this ugly hack by clearner API in the next cycle.
see:
https://lore.kernel.org/netdev/20241112001308.58355-1-kuniyu@amazon.com/
---
 net/tipc/udp_media.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 439f75539977..10986b283ac8 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -673,6 +673,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 	struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
 	u8 node_id[NODE_ID_LEN] = {0,};
 	struct net_device *dev;
+	struct sock *sk;
 	int rmcast = 0;
 
 	ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
@@ -792,6 +793,12 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
 	if (err)
 		goto free;
 
+	sk = ub->ubsock->sk;
+	__netns_tracker_free(net, &sk->ns_tracker, false);
+	sk->sk_net_refcnt = 1;
+	get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
+	sock_inuse_add(net, 1);
+
 	return 0;
 
 free:
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Eric Dumazet]

On Tue, Nov 26, 2024 at 7:14 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> syzkaller reported a use-after-free of kernel UDP socket in
> cleanup_bearer() without repro. [0][1]
>
> When bearer_disable() calls tipc_udp_disable(), cleanup of the kernel
> UDP socket is deferred by work calling cleanup_bearer().
>
> Since the cited commit, however, the socket's netns might not be alive
> when the work is executed, resulting in use-after-free.
>
> Let's hold netns for the kernel UDP socket when created.
>
> Note that we can't call get_net() before scheduling the work and call
> put_net() in cleanup_bearer() because bearer_disable() could be called
> from pernet_operations.exit():
>
>   tipc_exit_net
>   `- tipc_net_stop
>      `- tipc_bearer_stop
>         `- bearer_disable
>
> [0]:
> ref_tracker: net notrefcnt@000000009b3d1faf has 1/1 users at
>      sk_alloc+0x438/0x608
>      inet_create+0x4c8/0xcb0
>      __sock_create+0x350/0x6b8
>      sock_create_kern+0x58/0x78
>      udp_sock_create4+0x68/0x398
>      udp_sock_create+0x88/0xc8
>      tipc_udp_enable+0x5e8/0x848
>      __tipc_nl_bearer_enable+0x84c/0xed8
>      tipc_nl_bearer_enable+0x38/0x60
>      genl_family_rcv_msg_doit+0x170/0x248
>      genl_rcv_msg+0x400/0x5b0
>      netlink_rcv_skb+0x1dc/0x398
>      genl_rcv+0x44/0x68
>      netlink_unicast+0x678/0x8b0
>      netlink_sendmsg+0x5e4/0x898
>      ____sys_sendmsg+0x500/0x830
>
> [1]:
> BUG: KMSAN: use-after-free in udp_hashslot include/net/udp.h:85 [inline]
> BUG: KMSAN: use-after-free in udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
>  udp_hashslot include/net/udp.h:85 [inline]
>  udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
>  sk_common_release+0xaf/0x3f0 net/core/sock.c:3820
>  inet_release+0x1e0/0x260 net/ipv4/af_inet.c:437
>  inet6_release+0x6f/0xd0 net/ipv6/af_inet6.c:489
>  __sock_release net/socket.c:658 [inline]
>  sock_release+0xa0/0x210 net/socket.c:686
>  cleanup_bearer+0x42d/0x4c0 net/tipc/udp_media.c:819
>  process_one_work kernel/workqueue.c:3229 [inline]
>  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
>  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
>  kthread+0x531/0x6b0 kernel/kthread.c:389
>  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
>
> Uninit was created at:
>  slab_free_hook mm/slub.c:2269 [inline]
>  slab_free mm/slub.c:4580 [inline]
>  kmem_cache_free+0x207/0xc40 mm/slub.c:4682
>  net_free net/core/net_namespace.c:454 [inline]
>  cleanup_net+0x16f2/0x19d0 net/core/net_namespace.c:647
>  process_one_work kernel/workqueue.c:3229 [inline]
>  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
>  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
>  kthread+0x531/0x6b0 kernel/kthread.c:389
>  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
>  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
>
> CPU: 0 UID: 0 PID: 54 Comm: kworker/0:2 Not tainted 6.12.0-rc1-00131-gf66ebf37d69c #7 91723d6f74857f70725e1583cba3cf4adc716cfa
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> Workqueue: events cleanup_bearer
>
> Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> I'll remove this ugly hack by clearner API in the next cycle.
> see:
> https://lore.kernel.org/netdev/20241112001308.58355-1-kuniyu@amazon.com/
> ---
>  net/tipc/udp_media.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
> index 439f75539977..10986b283ac8 100644
> --- a/net/tipc/udp_media.c
> +++ b/net/tipc/udp_media.c
> @@ -673,6 +673,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
>         struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
>         u8 node_id[NODE_ID_LEN] = {0,};
>         struct net_device *dev;
> +       struct sock *sk;
>         int rmcast = 0;
>
>         ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
> @@ -792,6 +793,12 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
>         if (err)
>                 goto free;
>
> +       sk = ub->ubsock->sk;
> +       __netns_tracker_free(net, &sk->ns_tracker, false);
> +       sk->sk_net_refcnt = 1;
> +       get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
> +       sock_inuse_add(net, 1);
> +
>         return 0;

I think 'kernel sockets' were not refcounted to allow the netns to be removed.

Otherwise, what would tipc_bearer_stop() be needed ?

tipc_exit_net(struct net *net)  // can only be called when all refcnt
have been released
 -> tipc_net_stop()
  -> tipc_bearer_stop()
    -> bearer_disable()
     -> tipc_udp_disable()
       -> INIT_WORK(&ub->work, cleanup_bearer); schedule_work(&ub->work);

Re: [PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Tue, 26 Nov 2024 11:53:07 +0100
> On Tue, Nov 26, 2024 at 7:14 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> >
> > syzkaller reported a use-after-free of kernel UDP socket in
> > cleanup_bearer() without repro. [0][1]
> >
> > When bearer_disable() calls tipc_udp_disable(), cleanup of the kernel
> > UDP socket is deferred by work calling cleanup_bearer().
> >
> > Since the cited commit, however, the socket's netns might not be alive
> > when the work is executed, resulting in use-after-free.
> >
> > Let's hold netns for the kernel UDP socket when created.
> >
> > Note that we can't call get_net() before scheduling the work and call
> > put_net() in cleanup_bearer() because bearer_disable() could be called
> > from pernet_operations.exit():
> >
> >   tipc_exit_net
> >   `- tipc_net_stop
> >      `- tipc_bearer_stop
> >         `- bearer_disable
> >
> > [0]:
> > ref_tracker: net notrefcnt@000000009b3d1faf has 1/1 users at
> >      sk_alloc+0x438/0x608
> >      inet_create+0x4c8/0xcb0
> >      __sock_create+0x350/0x6b8
> >      sock_create_kern+0x58/0x78
> >      udp_sock_create4+0x68/0x398
> >      udp_sock_create+0x88/0xc8
> >      tipc_udp_enable+0x5e8/0x848
> >      __tipc_nl_bearer_enable+0x84c/0xed8
> >      tipc_nl_bearer_enable+0x38/0x60
> >      genl_family_rcv_msg_doit+0x170/0x248
> >      genl_rcv_msg+0x400/0x5b0
> >      netlink_rcv_skb+0x1dc/0x398
> >      genl_rcv+0x44/0x68
> >      netlink_unicast+0x678/0x8b0
> >      netlink_sendmsg+0x5e4/0x898
> >      ____sys_sendmsg+0x500/0x830
> >
> > [1]:
> > BUG: KMSAN: use-after-free in udp_hashslot include/net/udp.h:85 [inline]
> > BUG: KMSAN: use-after-free in udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
> >  udp_hashslot include/net/udp.h:85 [inline]
> >  udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
> >  sk_common_release+0xaf/0x3f0 net/core/sock.c:3820
> >  inet_release+0x1e0/0x260 net/ipv4/af_inet.c:437
> >  inet6_release+0x6f/0xd0 net/ipv6/af_inet6.c:489
> >  __sock_release net/socket.c:658 [inline]
> >  sock_release+0xa0/0x210 net/socket.c:686
> >  cleanup_bearer+0x42d/0x4c0 net/tipc/udp_media.c:819
> >  process_one_work kernel/workqueue.c:3229 [inline]
> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
> >  kthread+0x531/0x6b0 kernel/kthread.c:389
> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
> >
> > Uninit was created at:
> >  slab_free_hook mm/slub.c:2269 [inline]
> >  slab_free mm/slub.c:4580 [inline]
> >  kmem_cache_free+0x207/0xc40 mm/slub.c:4682
> >  net_free net/core/net_namespace.c:454 [inline]
> >  cleanup_net+0x16f2/0x19d0 net/core/net_namespace.c:647
> >  process_one_work kernel/workqueue.c:3229 [inline]
> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
> >  kthread+0x531/0x6b0 kernel/kthread.c:389
> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
> >
> > CPU: 0 UID: 0 PID: 54 Comm: kworker/0:2 Not tainted 6.12.0-rc1-00131-gf66ebf37d69c #7 91723d6f74857f70725e1583cba3cf4adc716cfa
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> > Workqueue: events cleanup_bearer
> >
> > Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
> > Reported-by: syzkaller <syzkaller@googlegroups.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> > I'll remove this ugly hack by clearner API in the next cycle.
> > see:
> > https://lore.kernel.org/netdev/20241112001308.58355-1-kuniyu@amazon.com/
> > ---
> >  net/tipc/udp_media.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
> > index 439f75539977..10986b283ac8 100644
> > --- a/net/tipc/udp_media.c
> > +++ b/net/tipc/udp_media.c
> > @@ -673,6 +673,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> >         struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
> >         u8 node_id[NODE_ID_LEN] = {0,};
> >         struct net_device *dev;
> > +       struct sock *sk;
> >         int rmcast = 0;
> >
> >         ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
> > @@ -792,6 +793,12 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> >         if (err)
> >                 goto free;
> >
> > +       sk = ub->ubsock->sk;
> > +       __netns_tracker_free(net, &sk->ns_tracker, false);
> > +       sk->sk_net_refcnt = 1;
> > +       get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
> > +       sock_inuse_add(net, 1);
> > +
> >         return 0;
> 
> I think 'kernel sockets' were not refcounted to allow the netns to be removed.
> 
> Otherwise, what would tipc_bearer_stop() be needed ?

Interestingly, the delayed cleanup exists since the udp media support
was added in d0f91938bede2, and it's 2 months earlier than 26abe14379f8
that drops netns refcnt for kernel sockets.

So I thought the udp bearer did not assume bearer_disable() was called
from the __net_exit path, it could be simply wrong though.

At least, the __net_exit path works for other media types.

@Erik Hugne, do you remember any context above ?


> 
> tipc_exit_net(struct net *net)  // can only be called when all refcnt
> have been released
>  -> tipc_net_stop()
>   -> tipc_bearer_stop()
>     -> bearer_disable()
>      -> tipc_udp_disable()
>        -> INIT_WORK(&ub->work, cleanup_bearer); schedule_work(&ub->work);
> 

Re: [PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Eric W. Biederman]

Kuniyuki Iwashima <kuniyu@amazon.com> writes:

> From: Eric Dumazet <edumazet@google.com>
> Date: Tue, 26 Nov 2024 11:53:07 +0100
>> On Tue, Nov 26, 2024 at 7:14 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>> >
>> > syzkaller reported a use-after-free of kernel UDP socket in
>> > cleanup_bearer() without repro. [0][1]
>> >
>> > When bearer_disable() calls tipc_udp_disable(), cleanup of the kernel
>> > UDP socket is deferred by work calling cleanup_bearer().
>> >
>> > Since the cited commit, however, the socket's netns might not be alive
>> > when the work is executed, resulting in use-after-free.
>> >
>> > Let's hold netns for the kernel UDP socket when created.
>> >
>> > Note that we can't call get_net() before scheduling the work and call
>> > put_net() in cleanup_bearer() because bearer_disable() could be called
>> > from pernet_operations.exit():
>> >
>> >   tipc_exit_net
>> >   `- tipc_net_stop
>> >      `- tipc_bearer_stop
>> >         `- bearer_disable
>> >
>> > [0]:
>> > ref_tracker: net notrefcnt@000000009b3d1faf has 1/1 users at
>> >      sk_alloc+0x438/0x608
>> >      inet_create+0x4c8/0xcb0
>> >      __sock_create+0x350/0x6b8
>> >      sock_create_kern+0x58/0x78
>> >      udp_sock_create4+0x68/0x398
>> >      udp_sock_create+0x88/0xc8
>> >      tipc_udp_enable+0x5e8/0x848
>> >      __tipc_nl_bearer_enable+0x84c/0xed8
>> >      tipc_nl_bearer_enable+0x38/0x60
>> >      genl_family_rcv_msg_doit+0x170/0x248
>> >      genl_rcv_msg+0x400/0x5b0
>> >      netlink_rcv_skb+0x1dc/0x398
>> >      genl_rcv+0x44/0x68
>> >      netlink_unicast+0x678/0x8b0
>> >      netlink_sendmsg+0x5e4/0x898
>> >      ____sys_sendmsg+0x500/0x830
>> >
>> > [1]:
>> > BUG: KMSAN: use-after-free in udp_hashslot include/net/udp.h:85 [inline]
>> > BUG: KMSAN: use-after-free in udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
>> >  udp_hashslot include/net/udp.h:85 [inline]
>> >  udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
>> >  sk_common_release+0xaf/0x3f0 net/core/sock.c:3820
>> >  inet_release+0x1e0/0x260 net/ipv4/af_inet.c:437
>> >  inet6_release+0x6f/0xd0 net/ipv6/af_inet6.c:489
>> >  __sock_release net/socket.c:658 [inline]
>> >  sock_release+0xa0/0x210 net/socket.c:686
>> >  cleanup_bearer+0x42d/0x4c0 net/tipc/udp_media.c:819
>> >  process_one_work kernel/workqueue.c:3229 [inline]
>> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
>> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
>> >  kthread+0x531/0x6b0 kernel/kthread.c:389
>> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
>> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
>> >
>> > Uninit was created at:
>> >  slab_free_hook mm/slub.c:2269 [inline]
>> >  slab_free mm/slub.c:4580 [inline]
>> >  kmem_cache_free+0x207/0xc40 mm/slub.c:4682
>> >  net_free net/core/net_namespace.c:454 [inline]
>> >  cleanup_net+0x16f2/0x19d0 net/core/net_namespace.c:647
>> >  process_one_work kernel/workqueue.c:3229 [inline]
>> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
>> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
>> >  kthread+0x531/0x6b0 kernel/kthread.c:389
>> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
>> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
>> >
>> > CPU: 0 UID: 0 PID: 54 Comm: kworker/0:2 Not tainted 6.12.0-rc1-00131-gf66ebf37d69c #7 91723d6f74857f70725e1583cba3cf4adc716cfa
>> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
>> > Workqueue: events cleanup_bearer
>> >
>> > Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
>> > Reported-by: syzkaller <syzkaller@googlegroups.com>
>> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
>> > ---
>> > I'll remove this ugly hack by clearner API in the next cycle.
>> > see:
>> > https://lore.kernel.org/netdev/20241112001308.58355-1-kuniyu@amazon.com/
>> > ---
>> >  net/tipc/udp_media.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
>> > index 439f75539977..10986b283ac8 100644
>> > --- a/net/tipc/udp_media.c
>> > +++ b/net/tipc/udp_media.c
>> > @@ -673,6 +673,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
>> >         struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
>> >         u8 node_id[NODE_ID_LEN] = {0,};
>> >         struct net_device *dev;
>> > +       struct sock *sk;
>> >         int rmcast = 0;
>> >
>> >         ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
>> > @@ -792,6 +793,12 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
>> >         if (err)
>> >                 goto free;
>> >
>> > +       sk = ub->ubsock->sk;
>> > +       __netns_tracker_free(net, &sk->ns_tracker, false);
>> > +       sk->sk_net_refcnt = 1;
>> > +       get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
>> > +       sock_inuse_add(net, 1);
>> > +
>> >         return 0;
>> 
>> I think 'kernel sockets' were not refcounted to allow the netns to be removed.
>> 
>> Otherwise, what would tipc_bearer_stop() be needed ?
>
> Interestingly, the delayed cleanup exists since the udp media support
> was added in d0f91938bede2, and it's 2 months earlier than 26abe14379f8
> that drops netns refcnt for kernel sockets.

Just for reference commit 26abe14379f8 ("net: Modify sk_alloc to not
reference count the netns of kernel sockets.") doesn't ``drop'' the
netns refcnt for kernel sockets.  It changes the code so the refcnt is
not taken.  You will see in that commit a bunch of sk_change_net calls
which if memory serves are where the refcnt was previously dropped.

> So I thought the udp bearer did not assume bearer_disable() was called
> from the __net_exit path, it could be simply wrong though.
>
> At least, the __net_exit path works for other media types.

For the most part.  The network filesystem has been seeing similar
issues lately.  I suspect there is something (maybe just syzkaller)
that is making old bugs more likely to appear lately.

> @Erik Hugne, do you remember any context above ?
>
>
>> 
>> tipc_exit_net(struct net *net)  // can only be called when all refcnt
>> have been released
>>  -> tipc_net_stop()
>>   -> tipc_bearer_stop()
>>     -> bearer_disable()
>>      -> tipc_udp_disable()
>>        -> INIT_WORK(&ub->work, cleanup_bearer); schedule_work(&ub->work);
>> 

That schedule_work definitely looks like it will start running after
the network namespace and probably the entire kernel socket
has been released.

Eric

Re: [PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Tue, 26 Nov 2024 11:53:07 +0100
> I think 'kernel sockets' were not refcounted to allow the netns to be removed.
> 
> Otherwise, what would tipc_bearer_stop() be needed ?
> 
> tipc_exit_net(struct net *net)  // can only be called when all refcnt
> have been released
>  -> tipc_net_stop()
>   -> tipc_bearer_stop()
>     -> bearer_disable()
>      -> tipc_udp_disable()
>        -> INIT_WORK(&ub->work, cleanup_bearer); schedule_work(&ub->work);

I noticed tipc_net_stop() waits for all works to be completed by
checking tipc_net(net)->wq_count, but it was decremented a bit
early in the work, so I'll post the following as v2:

---8<---
diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
index 10986b283ac8..ef3d8f71bde5 100644
--- a/net/tipc/udp_media.c
+++ b/net/tipc/udp_media.c
@@ -821,9 +821,9 @@ static void cleanup_bearer(struct work_struct *work)
 		kfree_rcu(rcast, rcu);
 	}
 
-	atomic_dec(&tipc_net(sock_net(ub->ubsock->sk))->wq_count);
 	dst_cache_destroy(&ub->rcast.dst_cache);
 	udp_tunnel_sock_release(ub->ubsock);
 	synchronize_net();
+	atomic_dec(&tipc_net(sock_net(ub->ubsock->sk))->wq_count);
 	kfree(ub);
 }
---8<---

Re: [PATCH v1 net] tipc: Fix use-after-free of kernel socket in cleanup_bearer().

[Kuniyuki Iwashima]

From: "Eric W. Biederman" <ebiederm@xmission.com>
Date: Tue, 26 Nov 2024 09:49:44 -0600
> Kuniyuki Iwashima <kuniyu@amazon.com> writes:
> 
> > From: Eric Dumazet <edumazet@google.com>
> > Date: Tue, 26 Nov 2024 11:53:07 +0100
> >> On Tue, Nov 26, 2024 at 7:14 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> >> >
> >> > syzkaller reported a use-after-free of kernel UDP socket in
> >> > cleanup_bearer() without repro. [0][1]
> >> >
> >> > When bearer_disable() calls tipc_udp_disable(), cleanup of the kernel
> >> > UDP socket is deferred by work calling cleanup_bearer().
> >> >
> >> > Since the cited commit, however, the socket's netns might not be alive
> >> > when the work is executed, resulting in use-after-free.
> >> >
> >> > Let's hold netns for the kernel UDP socket when created.
> >> >
> >> > Note that we can't call get_net() before scheduling the work and call
> >> > put_net() in cleanup_bearer() because bearer_disable() could be called
> >> > from pernet_operations.exit():
> >> >
> >> >   tipc_exit_net
> >> >   `- tipc_net_stop
> >> >      `- tipc_bearer_stop
> >> >         `- bearer_disable
> >> >
> >> > [0]:
> >> > ref_tracker: net notrefcnt@000000009b3d1faf has 1/1 users at
> >> >      sk_alloc+0x438/0x608
> >> >      inet_create+0x4c8/0xcb0
> >> >      __sock_create+0x350/0x6b8
> >> >      sock_create_kern+0x58/0x78
> >> >      udp_sock_create4+0x68/0x398
> >> >      udp_sock_create+0x88/0xc8
> >> >      tipc_udp_enable+0x5e8/0x848
> >> >      __tipc_nl_bearer_enable+0x84c/0xed8
> >> >      tipc_nl_bearer_enable+0x38/0x60
> >> >      genl_family_rcv_msg_doit+0x170/0x248
> >> >      genl_rcv_msg+0x400/0x5b0
> >> >      netlink_rcv_skb+0x1dc/0x398
> >> >      genl_rcv+0x44/0x68
> >> >      netlink_unicast+0x678/0x8b0
> >> >      netlink_sendmsg+0x5e4/0x898
> >> >      ____sys_sendmsg+0x500/0x830
> >> >
> >> > [1]:
> >> > BUG: KMSAN: use-after-free in udp_hashslot include/net/udp.h:85 [inline]
> >> > BUG: KMSAN: use-after-free in udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
> >> >  udp_hashslot include/net/udp.h:85 [inline]
> >> >  udp_lib_unhash+0x3b8/0x930 net/ipv4/udp.c:1979
> >> >  sk_common_release+0xaf/0x3f0 net/core/sock.c:3820
> >> >  inet_release+0x1e0/0x260 net/ipv4/af_inet.c:437
> >> >  inet6_release+0x6f/0xd0 net/ipv6/af_inet6.c:489
> >> >  __sock_release net/socket.c:658 [inline]
> >> >  sock_release+0xa0/0x210 net/socket.c:686
> >> >  cleanup_bearer+0x42d/0x4c0 net/tipc/udp_media.c:819
> >> >  process_one_work kernel/workqueue.c:3229 [inline]
> >> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
> >> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
> >> >  kthread+0x531/0x6b0 kernel/kthread.c:389
> >> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
> >> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
> >> >
> >> > Uninit was created at:
> >> >  slab_free_hook mm/slub.c:2269 [inline]
> >> >  slab_free mm/slub.c:4580 [inline]
> >> >  kmem_cache_free+0x207/0xc40 mm/slub.c:4682
> >> >  net_free net/core/net_namespace.c:454 [inline]
> >> >  cleanup_net+0x16f2/0x19d0 net/core/net_namespace.c:647
> >> >  process_one_work kernel/workqueue.c:3229 [inline]
> >> >  process_scheduled_works+0xcaf/0x1c90 kernel/workqueue.c:3310
> >> >  worker_thread+0xf6c/0x1510 kernel/workqueue.c:3391
> >> >  kthread+0x531/0x6b0 kernel/kthread.c:389
> >> >  ret_from_fork+0x60/0x80 arch/x86/kernel/process.c:147
> >> >  ret_from_fork_asm+0x11/0x20 arch/x86/entry/entry_64.S:244
> >> >
> >> > CPU: 0 UID: 0 PID: 54 Comm: kworker/0:2 Not tainted 6.12.0-rc1-00131-gf66ebf37d69c #7 91723d6f74857f70725e1583cba3cf4adc716cfa
> >> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> >> > Workqueue: events cleanup_bearer
> >> >
> >> > Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
> >> > Reported-by: syzkaller <syzkaller@googlegroups.com>
> >> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> >> > ---
> >> > I'll remove this ugly hack by clearner API in the next cycle.
> >> > see:
> >> > https://lore.kernel.org/netdev/20241112001308.58355-1-kuniyu@amazon.com/
> >> > ---
> >> >  net/tipc/udp_media.c | 7 +++++++
> >> >  1 file changed, 7 insertions(+)
> >> >
> >> > diff --git a/net/tipc/udp_media.c b/net/tipc/udp_media.c
> >> > index 439f75539977..10986b283ac8 100644
> >> > --- a/net/tipc/udp_media.c
> >> > +++ b/net/tipc/udp_media.c
> >> > @@ -673,6 +673,7 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> >> >         struct nlattr *opts[TIPC_NLA_UDP_MAX + 1];
> >> >         u8 node_id[NODE_ID_LEN] = {0,};
> >> >         struct net_device *dev;
> >> > +       struct sock *sk;
> >> >         int rmcast = 0;
> >> >
> >> >         ub = kzalloc(sizeof(*ub), GFP_ATOMIC);
> >> > @@ -792,6 +793,12 @@ static int tipc_udp_enable(struct net *net, struct tipc_bearer *b,
> >> >         if (err)
> >> >                 goto free;
> >> >
> >> > +       sk = ub->ubsock->sk;
> >> > +       __netns_tracker_free(net, &sk->ns_tracker, false);
> >> > +       sk->sk_net_refcnt = 1;
> >> > +       get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
> >> > +       sock_inuse_add(net, 1);
> >> > +
> >> >         return 0;
> >> 
> >> I think 'kernel sockets' were not refcounted to allow the netns to be removed.
> >> 
> >> Otherwise, what would tipc_bearer_stop() be needed ?
> >
> > Interestingly, the delayed cleanup exists since the udp media support
> > was added in d0f91938bede2, and it's 2 months earlier than 26abe14379f8
> > that drops netns refcnt for kernel sockets.
> 
> Just for reference commit 26abe14379f8 ("net: Modify sk_alloc to not
> reference count the netns of kernel sockets.") doesn't ``drop'' the
> netns refcnt for kernel sockets.  It changes the code so the refcnt is
> not taken.  You will see in that commit a bunch of sk_change_net calls
> which if memory serves are where the refcnt was previously dropped.
> 
> > So I thought the udp bearer did not assume bearer_disable() was called
> > from the __net_exit path, it could be simply wrong though.
> >
> > At least, the __net_exit path works for other media types.
> 
> For the most part.  The network filesystem has been seeing similar
> issues lately.  I suspect there is something (maybe just syzkaller)
> that is making old bugs more likely to appear lately.

It's not only syzkaller.

For example, mounting CIFS in k8s pod uncovered one of the issue.


> 
> > @Erik Hugne, do you remember any context above ?
> >
> >
> >> 
> >> tipc_exit_net(struct net *net)  // can only be called when all refcnt
> >> have been released
> >>  -> tipc_net_stop()
> >>   -> tipc_bearer_stop()
> >>     -> bearer_disable()
> >>      -> tipc_udp_disable()
> >>        -> INIT_WORK(&ub->work, cleanup_bearer); schedule_work(&ub->work);
> >> 
> 
> That schedule_work definitely looks like it will start running after
> the network namespace and probably the entire kernel socket
> has been released.
> 
> Eric