[PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[Liu Jian]

BUG: KASAN: slab-use-after-free in tcp_write_timer_handler+0x156/0x3e0
Read of size 1 at addr ffff888111f322cd by task swapper/0/0

CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.12.0-rc4-dirty #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
Call Trace:
 <IRQ>
 dump_stack_lvl+0x68/0xa0
 print_address_description.constprop.0+0x2c/0x3d0
 print_report+0xb4/0x270
 kasan_report+0xbd/0xf0
 tcp_write_timer_handler+0x156/0x3e0
 tcp_write_timer+0x66/0x170
 call_timer_fn+0xfb/0x1d0
 __run_timers+0x3f8/0x480
 run_timer_softirq+0x9b/0x100
 handle_softirqs+0x153/0x390
 __irq_exit_rcu+0x103/0x120
 irq_exit_rcu+0xe/0x20
 sysvec_apic_timer_interrupt+0x76/0x90
 </IRQ>
 <TASK>
 asm_sysvec_apic_timer_interrupt+0x1a/0x20
RIP: 0010:default_idle+0xf/0x20
Code: 4c 01 c7 4c 29 c2 e9 72 ff ff ff 90 90 90 90 90 90 90 90 90 90 90 90
 90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d 33 f8 25 00 fb f4 <fa> c3 cc cc cc
 cc 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90
RSP: 0018:ffffffffa2007e28 EFLAGS: 00000242
RAX: 00000000000f3b31 RBX: 1ffffffff4400fc7 RCX: ffffffffa09c3196
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff9f00590f
RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed102360835d
R10: ffff88811b041aeb R11: 0000000000000001 R12: 0000000000000000
R13: ffffffffa202d7c0 R14: 0000000000000000 R15: 00000000000147d0
 default_idle_call+0x6b/0xa0
 cpuidle_idle_call+0x1af/0x1f0
 do_idle+0xbc/0x130
 cpu_startup_entry+0x33/0x40
 rest_init+0x11f/0x210
 start_kernel+0x39a/0x420
 x86_64_start_reservations+0x18/0x30
 x86_64_start_kernel+0x97/0xa0
 common_startup_64+0x13e/0x141
 </TASK>

Allocated by task 595:
 kasan_save_stack+0x24/0x50
 kasan_save_track+0x14/0x30
 __kasan_slab_alloc+0x87/0x90
 kmem_cache_alloc_noprof+0x12b/0x3f0
 copy_net_ns+0x94/0x380
 create_new_namespaces+0x24c/0x500
 unshare_nsproxy_namespaces+0x75/0xf0
 ksys_unshare+0x24e/0x4f0
 __x64_sys_unshare+0x1f/0x30
 do_syscall_64+0x70/0x180
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Freed by task 100:
 kasan_save_stack+0x24/0x50
 kasan_save_track+0x14/0x30
 kasan_save_free_info+0x3b/0x60
 __kasan_slab_free+0x54/0x70
 kmem_cache_free+0x156/0x5d0
 cleanup_net+0x5d3/0x670
 process_one_work+0x776/0xa90
 worker_thread+0x2e2/0x560
 kthread+0x1a8/0x1f0
 ret_from_fork+0x34/0x60
 ret_from_fork_asm+0x1a/0x30

Reproduction script:

mkdir -p /mnt/nfsshare
mkdir -p /mnt/nfs/netns_1
mkfs.ext4 /dev/sdb
mount /dev/sdb /mnt/nfsshare
systemctl restart nfs-server
chmod 777 /mnt/nfsshare
exportfs -i -o rw,no_root_squash *:/mnt/nfsshare

ip netns add netns_1
ip link add name veth_1_peer type veth peer veth_1
ifconfig veth_1_peer 11.11.0.254 up
ip link set veth_1 netns netns_1
ip netns exec netns_1 ifconfig veth_1 11.11.0.1

ip netns exec netns_1 /root/iptables -A OUTPUT -d 11.11.0.254 -p tcp \
	--tcp-flags FIN FIN  -j DROP

(note: In my environment, a DESTROY_CLIENTID operation is always sent
 immediately, breaking the nfs tcp connection.)
ip netns exec netns_1 timeout -s 9 300 mount -t nfs -o proto=tcp,vers=4.1 \
	11.11.0.254:/mnt/nfsshare /mnt/nfs/netns_1

ip netns del netns_1

The reason here is that the tcp socket in netns_1 (nfs side) has been
shutdown and closed (done in xs_destroy), but the FIN message (with ack)
is discarded, and the nfsd side keeps sending retransmission messages.
As a result, when the tcp sock in netns_1 processes the received message,
it sends the message (FIN message) in the sending queue, and the tcp timer
is re-established. When the network namespace is deleted, the net structure
accessed by tcp's timer handler function causes problems.

To fix this problem:
Add the sock_create_kern_getnet() helper function, add the get_net()
 operation for the kernel socket.

Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
Signed-off-by: Liu Jian <liujian56@huawei.com>
---
v1: https://lore.kernel.org/all/20241024015543.568476-1-liujian56@huawei.com/
v1->v2: change to get netns reference count.
 include/linux/net.h   |  1 +
 net/socket.c          | 28 ++++++++++++++++++++++++++++
 net/sunrpc/svcsock.c  |  2 +-
 net/sunrpc/xprtsock.c |  2 +-
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index b75bc534c1b3..58216da3b62c 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -255,6 +255,7 @@ int __sock_create(struct net *net, int family, int type, int proto,
 		  struct socket **res, int kern);
 int sock_create(int family, int type, int proto, struct socket **res);
 int sock_create_kern(struct net *net, int family, int type, int proto, struct socket **res);
+int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res);
 int sock_create_lite(int family, int type, int proto, struct socket **res);
 struct socket *sock_alloc(void);
 void sock_release(struct socket *sock);
diff --git a/net/socket.c b/net/socket.c
index 042451f01c65..e64a02445b1a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, int type, int protocol, struct
 }
 EXPORT_SYMBOL(sock_create_kern);
 
+int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res)
+{
+	struct sock *sk;
+	int ret;
+
+	if (!maybe_get_net(net))
+		return -EINVAL;
+
+	ret = sock_create_kern(net, family, type, proto, res);
+	if (ret < 0) {
+		put_net(net);
+		return ret;
+	}
+
+	sk = (*res)->sk;
+	lock_sock(sk);
+	/* Update ns_tracker to current stack trace and refcounted tracker */
+	__netns_tracker_free(net, &sk->ns_tracker, false);
+
+	sk->sk_net_refcnt = 1;
+	netns_tracker_alloc(net, &sk->ns_tracker, GFP_KERNEL);
+	sock_inuse_add(net, 1);
+	release_sock(sk);
+
+	return ret;
+}
+EXPORT_SYMBOL(sock_create_kern_getnet);
+
 static struct socket *__sys_socket_create(int family, int type, int protocol)
 {
 	struct socket *sock;
diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
index 825ec5357691..6f272013fd9b 100644
--- a/net/sunrpc/svcsock.c
+++ b/net/sunrpc/svcsock.c
@@ -1526,7 +1526,7 @@ static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
 		return ERR_PTR(-EINVAL);
 	}
 
-	error = __sock_create(net, family, type, protocol, &sock, 1);
+	error = sock_create_kern_getnet(net, family, type, protocol, &sock);
 	if (error < 0)
 		return ERR_PTR(error);
 
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 110749b85040..f7734ce5eec9 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -1925,7 +1925,7 @@ static struct socket *xs_create_sock(struct rpc_xprt *xprt,
 	struct socket *sock;
 	int err;
 
-	err = __sock_create(xprt->xprt_net, family, type, protocol, &sock, 1);
+	err = sock_create_kern_getnet(xprt->xprt_net, family, type, protocol, &sock);
 	if (err < 0) {
 		dprintk("RPC:       can't create %d transport socket (%d).\n",
 				protocol, -err);
-- 
2.34.1

Re: [PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[Kuniyuki Iwashima]

From: Liu Jian <liujian56@huawei.com>
Date: Wed, 30 Oct 2024 17:49:53 +0800
> BUG: KASAN: slab-use-after-free in tcp_write_timer_handler+0x156/0x3e0
> Read of size 1 at addr ffff888111f322cd by task swapper/0/0
> 
> CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.12.0-rc4-dirty #7
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
> Call Trace:
>  <IRQ>
>  dump_stack_lvl+0x68/0xa0
>  print_address_description.constprop.0+0x2c/0x3d0
>  print_report+0xb4/0x270
>  kasan_report+0xbd/0xf0
>  tcp_write_timer_handler+0x156/0x3e0
>  tcp_write_timer+0x66/0x170
>  call_timer_fn+0xfb/0x1d0
>  __run_timers+0x3f8/0x480
>  run_timer_softirq+0x9b/0x100
>  handle_softirqs+0x153/0x390
>  __irq_exit_rcu+0x103/0x120
>  irq_exit_rcu+0xe/0x20
>  sysvec_apic_timer_interrupt+0x76/0x90
>  </IRQ>
>  <TASK>
>  asm_sysvec_apic_timer_interrupt+0x1a/0x20
> RIP: 0010:default_idle+0xf/0x20
> Code: 4c 01 c7 4c 29 c2 e9 72 ff ff ff 90 90 90 90 90 90 90 90 90 90 90 90
>  90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d 33 f8 25 00 fb f4 <fa> c3 cc cc cc
>  cc 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90
> RSP: 0018:ffffffffa2007e28 EFLAGS: 00000242
> RAX: 00000000000f3b31 RBX: 1ffffffff4400fc7 RCX: ffffffffa09c3196
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff9f00590f
> RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed102360835d
> R10: ffff88811b041aeb R11: 0000000000000001 R12: 0000000000000000
> R13: ffffffffa202d7c0 R14: 0000000000000000 R15: 00000000000147d0
>  default_idle_call+0x6b/0xa0
>  cpuidle_idle_call+0x1af/0x1f0
>  do_idle+0xbc/0x130
>  cpu_startup_entry+0x33/0x40
>  rest_init+0x11f/0x210
>  start_kernel+0x39a/0x420
>  x86_64_start_reservations+0x18/0x30
>  x86_64_start_kernel+0x97/0xa0
>  common_startup_64+0x13e/0x141
>  </TASK>
> 
> Allocated by task 595:
>  kasan_save_stack+0x24/0x50
>  kasan_save_track+0x14/0x30
>  __kasan_slab_alloc+0x87/0x90
>  kmem_cache_alloc_noprof+0x12b/0x3f0
>  copy_net_ns+0x94/0x380
>  create_new_namespaces+0x24c/0x500
>  unshare_nsproxy_namespaces+0x75/0xf0
>  ksys_unshare+0x24e/0x4f0
>  __x64_sys_unshare+0x1f/0x30
>  do_syscall_64+0x70/0x180
>  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> 
> Freed by task 100:
>  kasan_save_stack+0x24/0x50
>  kasan_save_track+0x14/0x30
>  kasan_save_free_info+0x3b/0x60
>  __kasan_slab_free+0x54/0x70
>  kmem_cache_free+0x156/0x5d0
>  cleanup_net+0x5d3/0x670
>  process_one_work+0x776/0xa90
>  worker_thread+0x2e2/0x560
>  kthread+0x1a8/0x1f0
>  ret_from_fork+0x34/0x60
>  ret_from_fork_asm+0x1a/0x30
> 
> Reproduction script:
> 
> mkdir -p /mnt/nfsshare
> mkdir -p /mnt/nfs/netns_1
> mkfs.ext4 /dev/sdb
> mount /dev/sdb /mnt/nfsshare
> systemctl restart nfs-server
> chmod 777 /mnt/nfsshare
> exportfs -i -o rw,no_root_squash *:/mnt/nfsshare
> 
> ip netns add netns_1
> ip link add name veth_1_peer type veth peer veth_1
> ifconfig veth_1_peer 11.11.0.254 up
> ip link set veth_1 netns netns_1
> ip netns exec netns_1 ifconfig veth_1 11.11.0.1
> 
> ip netns exec netns_1 /root/iptables -A OUTPUT -d 11.11.0.254 -p tcp \
> 	--tcp-flags FIN FIN  -j DROP
> 
> (note: In my environment, a DESTROY_CLIENTID operation is always sent
>  immediately, breaking the nfs tcp connection.)
> ip netns exec netns_1 timeout -s 9 300 mount -t nfs -o proto=tcp,vers=4.1 \
> 	11.11.0.254:/mnt/nfsshare /mnt/nfs/netns_1
> 
> ip netns del netns_1
> 
> The reason here is that the tcp socket in netns_1 (nfs side) has been
> shutdown and closed (done in xs_destroy), but the FIN message (with ack)
> is discarded, and the nfsd side keeps sending retransmission messages.
> As a result, when the tcp sock in netns_1 processes the received message,
> it sends the message (FIN message) in the sending queue, and the tcp timer
> is re-established. When the network namespace is deleted, the net structure
> accessed by tcp's timer handler function causes problems.
> 
> To fix this problem:
> Add the sock_create_kern_getnet() helper function, add the get_net()
>  operation for the kernel socket.
> 
> Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
> Signed-off-by: Liu Jian <liujian56@huawei.com>
> ---
> v1: https://lore.kernel.org/all/20241024015543.568476-1-liujian56@huawei.com/
> v1->v2: change to get netns reference count.
>  include/linux/net.h   |  1 +
>  net/socket.c          | 28 ++++++++++++++++++++++++++++
>  net/sunrpc/svcsock.c  |  2 +-
>  net/sunrpc/xprtsock.c |  2 +-
>  4 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index b75bc534c1b3..58216da3b62c 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -255,6 +255,7 @@ int __sock_create(struct net *net, int family, int type, int proto,
>  		  struct socket **res, int kern);
>  int sock_create(int family, int type, int proto, struct socket **res);
>  int sock_create_kern(struct net *net, int family, int type, int proto, struct socket **res);
> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res);
>  int sock_create_lite(int family, int type, int proto, struct socket **res);
>  struct socket *sock_alloc(void);
>  void sock_release(struct socket *sock);
> diff --git a/net/socket.c b/net/socket.c
> index 042451f01c65..e64a02445b1a 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, int type, int protocol, struct
>  }
>  EXPORT_SYMBOL(sock_create_kern);
>  
> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res)
> +{
> +	struct sock *sk;
> +	int ret;
> +
> +	if (!maybe_get_net(net))
> +		return -EINVAL;

Is this really safe ?

IIUC, maybe_get_net() is safe for a net only when it is fetched under
RCU, then rcu_read_lock() prevents cleanup_net() from reusing the net
by rcu_barrier().

Otherwise, there should be a small chance that the same slab object is
reused for another netns between fetching the net and reaching here.

svc_create_socket() is called much later after the netns is fetched,
and _svc_xprt_create() calls try_module_get() before ->xpo_create().
So, it seems the path is not under RCU and maybe_get_net() must be
called much earlier by each call site.

For this reason, when I write a patch for the same issue in CIFS,
I delayed put_net() to cifsd kthread so that the netns refcnt taken
for each CIFS server info lives until the last __sock_create() attempt
from cifsd.

https://lore.kernel.org/linux-cifs/20241102212438.76691-1-kuniyu@amazon.com/


> +
> +	ret = sock_create_kern(net, family, type, proto, res);
> +	if (ret < 0) {
> +		put_net(net);
> +		return ret;
> +	}
> +
> +	sk = (*res)->sk;
> +	lock_sock(sk);
> +	/* Update ns_tracker to current stack trace and refcounted tracker */
> +	__netns_tracker_free(net, &sk->ns_tracker, false);
> +
> +	sk->sk_net_refcnt = 1;
> +	netns_tracker_alloc(net, &sk->ns_tracker, GFP_KERNEL);
> +	sock_inuse_add(net, 1);
> +	release_sock(sk);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(sock_create_kern_getnet);
> +
>  static struct socket *__sys_socket_create(int family, int type, int protocol)
>  {
>  	struct socket *sock;
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index 825ec5357691..6f272013fd9b 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -1526,7 +1526,7 @@ static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	error = __sock_create(net, family, type, protocol, &sock, 1);
> +	error = sock_create_kern_getnet(net, family, type, protocol, &sock);
>  	if (error < 0)
>  		return ERR_PTR(error);
>  
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 110749b85040..f7734ce5eec9 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1925,7 +1925,7 @@ static struct socket *xs_create_sock(struct rpc_xprt *xprt,
>  	struct socket *sock;
>  	int err;
>  
> -	err = __sock_create(xprt->xprt_net, family, type, protocol, &sock, 1);
> +	err = sock_create_kern_getnet(xprt->xprt_net, family, type, protocol, &sock);
>  	if (err < 0) {
>  		dprintk("RPC:       can't create %d transport socket (%d).\n",
>  				protocol, -err);
> -- 
> 2.34.1

Re: [PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[liujian (CE)]

在 2024/11/3 12:09, Kuniyuki Iwashima 写道:
> From: Liu Jian <liujian56@huawei.com>
> Date: Wed, 30 Oct 2024 17:49:53 +0800
>> BUG: KASAN: slab-use-after-free in tcp_write_timer_handler+0x156/0x3e0
>> Read of size 1 at addr ffff888111f322cd by task swapper/0/0
>>
>> CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.12.0-rc4-dirty #7
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
>> Call Trace:
>>   <IRQ>
>>   dump_stack_lvl+0x68/0xa0
>>   print_address_description.constprop.0+0x2c/0x3d0
>>   print_report+0xb4/0x270
>>   kasan_report+0xbd/0xf0
>>   tcp_write_timer_handler+0x156/0x3e0
>>   tcp_write_timer+0x66/0x170
>>   call_timer_fn+0xfb/0x1d0
>>   __run_timers+0x3f8/0x480
>>   run_timer_softirq+0x9b/0x100
>>   handle_softirqs+0x153/0x390
>>   __irq_exit_rcu+0x103/0x120
>>   irq_exit_rcu+0xe/0x20
>>   sysvec_apic_timer_interrupt+0x76/0x90
>>   </IRQ>
>>   <TASK>
>>   asm_sysvec_apic_timer_interrupt+0x1a/0x20
>> RIP: 0010:default_idle+0xf/0x20
>> Code: 4c 01 c7 4c 29 c2 e9 72 ff ff ff 90 90 90 90 90 90 90 90 90 90 90 90
>>   90 90 90 90 f3 0f 1e fa 66 90 0f 00 2d 33 f8 25 00 fb f4 <fa> c3 cc cc cc
>>   cc 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90
>> RSP: 0018:ffffffffa2007e28 EFLAGS: 00000242
>> RAX: 00000000000f3b31 RBX: 1ffffffff4400fc7 RCX: ffffffffa09c3196
>> RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff9f00590f
>> RBP: 0000000000000000 R08: 0000000000000001 R09: ffffed102360835d
>> R10: ffff88811b041aeb R11: 0000000000000001 R12: 0000000000000000
>> R13: ffffffffa202d7c0 R14: 0000000000000000 R15: 00000000000147d0
>>   default_idle_call+0x6b/0xa0
>>   cpuidle_idle_call+0x1af/0x1f0
>>   do_idle+0xbc/0x130
>>   cpu_startup_entry+0x33/0x40
>>   rest_init+0x11f/0x210
>>   start_kernel+0x39a/0x420
>>   x86_64_start_reservations+0x18/0x30
>>   x86_64_start_kernel+0x97/0xa0
>>   common_startup_64+0x13e/0x141
>>   </TASK>
>>
>> Allocated by task 595:
>>   kasan_save_stack+0x24/0x50
>>   kasan_save_track+0x14/0x30
>>   __kasan_slab_alloc+0x87/0x90
>>   kmem_cache_alloc_noprof+0x12b/0x3f0
>>   copy_net_ns+0x94/0x380
>>   create_new_namespaces+0x24c/0x500
>>   unshare_nsproxy_namespaces+0x75/0xf0
>>   ksys_unshare+0x24e/0x4f0
>>   __x64_sys_unshare+0x1f/0x30
>>   do_syscall_64+0x70/0x180
>>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>>
>> Freed by task 100:
>>   kasan_save_stack+0x24/0x50
>>   kasan_save_track+0x14/0x30
>>   kasan_save_free_info+0x3b/0x60
>>   __kasan_slab_free+0x54/0x70
>>   kmem_cache_free+0x156/0x5d0
>>   cleanup_net+0x5d3/0x670
>>   process_one_work+0x776/0xa90
>>   worker_thread+0x2e2/0x560
>>   kthread+0x1a8/0x1f0
>>   ret_from_fork+0x34/0x60
>>   ret_from_fork_asm+0x1a/0x30
>>
>> Reproduction script:
>>
>> mkdir -p /mnt/nfsshare
>> mkdir -p /mnt/nfs/netns_1
>> mkfs.ext4 /dev/sdb
>> mount /dev/sdb /mnt/nfsshare
>> systemctl restart nfs-server
>> chmod 777 /mnt/nfsshare
>> exportfs -i -o rw,no_root_squash *:/mnt/nfsshare
>>
>> ip netns add netns_1
>> ip link add name veth_1_peer type veth peer veth_1
>> ifconfig veth_1_peer 11.11.0.254 up
>> ip link set veth_1 netns netns_1
>> ip netns exec netns_1 ifconfig veth_1 11.11.0.1
>>
>> ip netns exec netns_1 /root/iptables -A OUTPUT -d 11.11.0.254 -p tcp \
>> 	--tcp-flags FIN FIN  -j DROP
>>
>> (note: In my environment, a DESTROY_CLIENTID operation is always sent
>>   immediately, breaking the nfs tcp connection.)
>> ip netns exec netns_1 timeout -s 9 300 mount -t nfs -o proto=tcp,vers=4.1 \
>> 	11.11.0.254:/mnt/nfsshare /mnt/nfs/netns_1
>>
>> ip netns del netns_1
>>
>> The reason here is that the tcp socket in netns_1 (nfs side) has been
>> shutdown and closed (done in xs_destroy), but the FIN message (with ack)
>> is discarded, and the nfsd side keeps sending retransmission messages.
>> As a result, when the tcp sock in netns_1 processes the received message,
>> it sends the message (FIN message) in the sending queue, and the tcp timer
>> is re-established. When the network namespace is deleted, the net structure
>> accessed by tcp's timer handler function causes problems.
>>
>> To fix this problem:
>> Add the sock_create_kern_getnet() helper function, add the get_net()
>>   operation for the kernel socket.
>>
>> Fixes: 26abe14379f8 ("net: Modify sk_alloc to not reference count the netns of kernel sockets.")
>> Signed-off-by: Liu Jian <liujian56@huawei.com>
>> ---
>> v1: https://lore.kernel.org/all/20241024015543.568476-1-liujian56@huawei.com/
>> v1->v2: change to get netns reference count.
>>   include/linux/net.h   |  1 +
>>   net/socket.c          | 28 ++++++++++++++++++++++++++++
>>   net/sunrpc/svcsock.c  |  2 +-
>>   net/sunrpc/xprtsock.c |  2 +-
>>   4 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/net.h b/include/linux/net.h
>> index b75bc534c1b3..58216da3b62c 100644
>> --- a/include/linux/net.h
>> +++ b/include/linux/net.h
>> @@ -255,6 +255,7 @@ int __sock_create(struct net *net, int family, int type, int proto,
>>   		  struct socket **res, int kern);
>>   int sock_create(int family, int type, int proto, struct socket **res);
>>   int sock_create_kern(struct net *net, int family, int type, int proto, struct socket **res);
>> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res);
>>   int sock_create_lite(int family, int type, int proto, struct socket **res);
>>   struct socket *sock_alloc(void);
>>   void sock_release(struct socket *sock);
>> diff --git a/net/socket.c b/net/socket.c
>> index 042451f01c65..e64a02445b1a 100644
>> --- a/net/socket.c
>> +++ b/net/socket.c
>> @@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, int type, int protocol, struct
>>   }
>>   EXPORT_SYMBOL(sock_create_kern);
>>   
>> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res)
>> +{
>> +	struct sock *sk;
>> +	int ret;
>> +
>> +	if (!maybe_get_net(net))
>> +		return -EINVAL;
> 
> Is this really safe ?
> 
> IIUC, maybe_get_net() is safe for a net only when it is fetched under
> RCU, then rcu_read_lock() prevents cleanup_net() from reusing the net
> by rcu_barrier().
> 
> Otherwise, there should be a small chance that the same slab object is
> reused for another netns between fetching the net and reaching here.
> 
> svc_create_socket() is called much later after the netns is fetched,
> and _svc_xprt_create() calls try_module_get() before ->xpo_create().
> So, it seems the path is not under RCU and maybe_get_net() must be
> called much earlier by each call site.
> 
> For this reason, when I write a patch for the same issue in CIFS,
> I delayed put_net() to cifsd kthread so that the netns refcnt taken
> for each CIFS server info lives until the last __sock_create() attempt
> from cifsd.
> 
> https://lore.kernel.org/linux-cifs/20241102212438.76691-1-kuniyu@amazon.com/
> 
Okay, got it. thank you.
Looking at the nfs and nfsd processing flow, it seems that the call to 
__sock_create() to create a TCP socket is always after the mount 
operation get_net(). So it should be fine to use get_net() directly. So 
here I'm going to change may_get_net() to get_net(), move 
sock_create_kern_getnet() to the sunrpc module, and rename it to 
something more appropriate. Is that okay?
> 
>> +
>> +	ret = sock_create_kern(net, family, type, proto, res);
>> +	if (ret < 0) {
>> +		put_net(net);
>> +		return ret;
>> +	}
>> +
>> +	sk = (*res)->sk;
>> +	lock_sock(sk);
>> +	/* Update ns_tracker to current stack trace and refcounted tracker */
>> +	__netns_tracker_free(net, &sk->ns_tracker, false);
>> +
>> +	sk->sk_net_refcnt = 1;
>> +	netns_tracker_alloc(net, &sk->ns_tracker, GFP_KERNEL);
>> +	sock_inuse_add(net, 1);
>> +	release_sock(sk);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL(sock_create_kern_getnet);
>> +
>>   static struct socket *__sys_socket_create(int family, int type, int protocol)
>>   {
>>   	struct socket *sock;
>> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
>> index 825ec5357691..6f272013fd9b 100644
>> --- a/net/sunrpc/svcsock.c
>> +++ b/net/sunrpc/svcsock.c
>> @@ -1526,7 +1526,7 @@ static struct svc_xprt *svc_create_socket(struct svc_serv *serv,
>>   		return ERR_PTR(-EINVAL);
>>   	}
>>   
>> -	error = __sock_create(net, family, type, protocol, &sock, 1);
>> +	error = sock_create_kern_getnet(net, family, type, protocol, &sock);
>>   	if (error < 0)
>>   		return ERR_PTR(error);
>>   
>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
>> index 110749b85040..f7734ce5eec9 100644
>> --- a/net/sunrpc/xprtsock.c
>> +++ b/net/sunrpc/xprtsock.c
>> @@ -1925,7 +1925,7 @@ static struct socket *xs_create_sock(struct rpc_xprt *xprt,
>>   	struct socket *sock;
>>   	int err;
>>   
>> -	err = __sock_create(xprt->xprt_net, family, type, protocol, &sock, 1);
>> +	err = sock_create_kern_getnet(xprt->xprt_net, family, type, protocol, &sock);
>>   	if (err < 0) {
>>   		dprintk("RPC:       can't create %d transport socket (%d).\n",
>>   				protocol, -err);
>> -- 
>> 2.34.1
> 

Re: [PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[Kuniyuki Iwashima]

From: "liujian (CE)" <liujian56@huawei.com>
Date: Thu, 7 Nov 2024 20:03:40 +0800
> >> diff --git a/net/socket.c b/net/socket.c
> >> index 042451f01c65..e64a02445b1a 100644
> >> --- a/net/socket.c
> >> +++ b/net/socket.c
> >> @@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, int type, int protocol, struct
> >>   }
> >>   EXPORT_SYMBOL(sock_create_kern);
> >>   
> >> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res)
> >> +{
> >> +	struct sock *sk;
> >> +	int ret;
> >> +
> >> +	if (!maybe_get_net(net))
> >> +		return -EINVAL;
> > 
> > Is this really safe ?
> > 
> > IIUC, maybe_get_net() is safe for a net only when it is fetched under
> > RCU, then rcu_read_lock() prevents cleanup_net() from reusing the net
> > by rcu_barrier().
> > 
> > Otherwise, there should be a small chance that the same slab object is
> > reused for another netns between fetching the net and reaching here.
> > 
> > svc_create_socket() is called much later after the netns is fetched,
> > and _svc_xprt_create() calls try_module_get() before ->xpo_create().
> > So, it seems the path is not under RCU and maybe_get_net() must be
> > called much earlier by each call site.
> > 
> > For this reason, when I write a patch for the same issue in CIFS,
> > I delayed put_net() to cifsd kthread so that the netns refcnt taken
> > for each CIFS server info lives until the last __sock_create() attempt
> > from cifsd.
> > 
> > https://lore.kernel.org/linux-cifs/20241102212438.76691-1-kuniyu@amazon.com/
> > 
> Okay, got it. thank you.
> Looking at the nfs and nfsd processing flow, it seems that the call to 
> __sock_create() to create a TCP socket is always after the mount 
> operation get_net(). So it should be fine to use get_net() directly.

Is there any chance that a concurrent unmount releases the
last refcount by put_net() while another thread trying to call
__sock_create() ?

CIFS was the case.


> So 
> here I'm going to change may_get_net() to get_net(), move 
> sock_create_kern_getnet() to the sunrpc module, and rename it to 
> something more appropriate. Is that okay?

Could you go without adding a helper and do the conversion in sunrpc
code as CIFS did ?

I plan to resurrect my patch and remove such socket conversion altogether
in the next cycle after the CIFS fix lands on net-next.

https://lore.kernel.org/netdev/20240227011041.97375-4-kuniyu@amazon.com/
https://github.com/q2ven/linux/commits/427_2
https://github.com/q2ven/linux/commit/2e54a8cc84f1e9ce60a0e4693c79a8e74c3dbeb9

I inspected all the callers of __sock_create() and friends, and all
__sock_create() can be replaced with sock_create_kern(), so I will
unexport __sock_create() and then add a new parameter hold_net to it.

Then, I'll rename sock_create_kern() to sock_create_net_noref() and add
a fat comment to catch in-kernel users attention so that they no longer
use _kern() API blindly without care about netns reference.  Also, I'll
add sock_create_net() and use it for MPTCP, SMC, CIFS, (and sunrpc) etc.

RDS uses maybe_net_get() but I think this is still buggy and I need
to check more.

Re: [PATCH net v2] sunrpc: fix one UAF issue caused by sunrpc kernel tcp socket

[liujian (CE)]

在 2024/11/8 4:59, Kuniyuki Iwashima 写道:
> From: "liujian (CE)" <liujian56@huawei.com>
> Date: Thu, 7 Nov 2024 20:03:40 +0800
>>>> diff --git a/net/socket.c b/net/socket.c
>>>> index 042451f01c65..e64a02445b1a 100644
>>>> --- a/net/socket.c
>>>> +++ b/net/socket.c
>>>> @@ -1651,6 +1651,34 @@ int sock_create_kern(struct net *net, int family, int type, int protocol, struct
>>>>    }
>>>>    EXPORT_SYMBOL(sock_create_kern);
>>>>    
>>>> +int sock_create_kern_getnet(struct net *net, int family, int type, int proto, struct socket **res)
>>>> +{
>>>> +	struct sock *sk;
>>>> +	int ret;
>>>> +
>>>> +	if (!maybe_get_net(net))
>>>> +		return -EINVAL;
>>>
>>> Is this really safe ?
>>>
>>> IIUC, maybe_get_net() is safe for a net only when it is fetched under
>>> RCU, then rcu_read_lock() prevents cleanup_net() from reusing the net
>>> by rcu_barrier().
>>>
>>> Otherwise, there should be a small chance that the same slab object is
>>> reused for another netns between fetching the net and reaching here.
>>>
>>> svc_create_socket() is called much later after the netns is fetched,
>>> and _svc_xprt_create() calls try_module_get() before ->xpo_create().
>>> So, it seems the path is not under RCU and maybe_get_net() must be
>>> called much earlier by each call site.
>>>
>>> For this reason, when I write a patch for the same issue in CIFS,
>>> I delayed put_net() to cifsd kthread so that the netns refcnt taken
>>> for each CIFS server info lives until the last __sock_create() attempt
>>> from cifsd.
>>>
>>> https://lore.kernel.org/linux-cifs/20241102212438.76691-1-kuniyu@amazon.com/
>>>
>> Okay, got it. thank you.
>> Looking at the nfs and nfsd processing flow, it seems that the call to
>> __sock_create() to create a TCP socket is always after the mount
>> operation get_net(). So it should be fine to use get_net() directly.
> 
> Is there any chance that a concurrent unmount releases the
> last refcount by put_net() while another thread trying to call
> __sock_create() ?
> 
> CIFS was the case.
> 
> 
>> So
>> here I'm going to change may_get_net() to get_net(), move
>> sock_create_kern_getnet() to the sunrpc module, and rename it to
>> something more appropriate. Is that okay?
> 
> Could you go without adding a helper and do the conversion in sunrpc
> code as CIFS did ?
> 
Ok, I will send v3 as you said.
Looking forward to your changes as described below.
Thank you.

> I plan to resurrect my patch and remove such socket conversion altogether
> in the next cycle after the CIFS fix lands on net-next.
> 
> https://lore.kernel.org/netdev/20240227011041.97375-4-kuniyu@amazon.com/
> https://github.com/q2ven/linux/commits/427_2
> https://github.com/q2ven/linux/commit/2e54a8cc84f1e9ce60a0e4693c79a8e74c3dbeb9
> 
> I inspected all the callers of __sock_create() and friends, and all
> __sock_create() can be replaced with sock_create_kern(), so I will
> unexport __sock_create() and then add a new parameter hold_net to it.
> 
> Then, I'll rename sock_create_kern() to sock_create_net_noref() and add
> a fat comment to catch in-kernel users attention so that they no longer
> use _kern() API blindly without care about netns reference.  Also, I'll
> add sock_create_net() and use it for MPTCP, SMC, CIFS, (and sunrpc) etc.
> 
> RDS uses maybe_net_get() but I think this is still buggy and I need
> to check more.