[PATCH v3] RDMA/siw: Fix early peer close handling on listeners

[PATCH v3] RDMA/siw: Fix early peer close handling on listeners

[Jaeyeong Lee]

A TCP child socket inherits the listener callbacks and sk_user_data before
kernel_accept() associates it with a new connection endpoint. If the peer
closes during this window, siw_cm_llp_state_change() queues a PEER_CLOSE
work item against the listening endpoint.

The worker releases that endpoint without removing it from the listener
list. A later listener teardown then walks the dangling entry, causing a
use-after-free in siw_destroy_listen(). KASAN reports:

  ==================================================================
  BUG: KASAN: slab-use-after-free in __list_del_entry_valid_or_report+0x32/0x1a0
  Read of size 8 at addr ffff888008f502c8 by task rping/59

  CPU: 0 UID: 0 PID: 59 Comm: rping Tainted: G    B               7.4.0-rc1-g6628e7d4c6b2 #1 PREEMPT(lazy)
  Tainted: [B]=BAD_PAGE
  Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
  Call Trace:
   <TASK>
   dump_stack_lvl+0x54/0x70
   print_address_description+0x73/0x1f0
   ? __list_del_entry_valid_or_report+0x32/0x1a0
   print_report+0x58/0x70
   kasan_report+0x117/0x150
   ? add_taint+0x43/0x90
   ? __list_del_entry_valid_or_report+0x32/0x1a0
   __list_del_entry_valid_or_report+0x32/0x1a0
   siw_destroy_listen+0xe1/0x630
   destroy_cm_id+0x25c/0x480
   ? srso_return_thunk+0x5/0x5f
   ? srso_return_thunk+0x5/0x5f
   ? lock_acquire+0xf7/0x220
   ? _destroy_id+0x2d1/0xe30
   iw_destroy_cm_id+0xf/0x80
   _destroy_id+0x6cc/0xe30
   _cma_cancel_listens+0x2ee/0x360
   _destroy_id+0x2ad/0xe30
   ucma_destroy_private_ctx+0xb7/0x9d0
   ? srso_return_thunk+0x5/0x5f
   ? do_raw_spin_unlock+0x4d/0x230
   ucma_close+0xd5/0x130
   __fput+0x31a/0x720
   fput_close_sync+0x91/0x110
   filp_close_sync+0x18/0x30
   put_files_struct+0x25a/0x280
   do_exit+0x70e/0x1d00
   ? lock_acquire+0xf7/0x220
   ? get_signal+0x21c/0xf60
   do_group_exit+0x19e/0x250
   get_signal+0xf20/0xf60
   ? futex_wait+0x99/0x230
   arch_do_signal_or_restart+0x46/0x420
   exit_to_user_mode_loop+0xbd/0x510
   do_syscall_64+0x2ed/0x3b0
   ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
   ? srso_return_thunk+0x5/0x5f
   ? trace_hardirqs_off+0x65/0x90
   entry_SYSCALL_64_after_hwframe+0x76/0x7e
  RIP: 0033:0x7f92984c6e51
  Code: Unable to access opcode bytes at 0x7f92984c6e27.
  RSP: 002b:00007ffeae782d10 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
  RAX: fffffffffffffe00 RBX: 000055fb4e9819b8 RCX: 00007f92984c6e51
  RDX: 0000000000000000 RSI: 0000000000000189 RDI: 000055fb4e9819b8
  RBP: 00007ffeae782d50 R08: 0000000000000000 R09: 00000000ffffffff
  R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
  R13: 0000000000000000 R14: 000055fb4e9819b8 R15: 000055fb4e97da25
   </TASK>

  Allocated by task 59:
   kasan_save_track+0x3e/0x80
   __kasan_kmalloc+0x72/0x90
   __kmalloc_cache_noprof+0x245/0x3c0
   siw_cep_alloc+0x4f/0x390
   siw_create_listen+0x39e/0xd60
   iw_cm_listen+0xfc/0x1d0
   rdma_listen+0x749/0xa90
   cma_listen_on_dev+0x404/0x760
   rdma_listen+0x3dc/0xa90
   ucma_listen+0xe2/0x180
   ucma_write+0x1bd/0x1e0
   vfs_write+0x1a9/0x8e0
   ksys_write+0xba/0x180
   do_syscall_64+0xee/0x3b0
   entry_SYSCALL_64_after_hwframe+0x76/0x7e

  Freed by task 30:
   kasan_save_track+0x3e/0x80
   kasan_save_free_info+0x40/0x50
   __kasan_slab_free+0x3a/0x60
   kfree+0x155/0x470
   siw_cm_work_handler+0x7af/0x2d40
   process_scheduled_works+0x970/0x12d0
   worker_thread+0x894/0xd20
   kthread+0x2db/0x3b0
   ret_from_fork+0x266/0x570
   ret_from_fork_asm+0x1a/0x30

  The buggy address belongs to the object at ffff888008f50200
   which belongs to the cache kmalloc-512 of size 512
  The buggy address is located 200 bytes inside of
   freed 512-byte region [ffff888008f50200, ffff888008f50400)

  The buggy address belongs to the physical page:
  page: refcount:0 mapcount:0 mapping:0000000000000000
  index:0xffff888008f51400 pfn:0x8f50
  head: order:2 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
  flags: 0x4000000000000240(workingset|head|zone=1)
  page_type: f5(slab)
  raw: 4000000000000240 ffff888005842040 ffff888005840d08 ffff888005840d08
  raw: ffff888008f51400 00000000000a0003 00000000f5000000 0000000000000000
  head: 4000000000000240 ffff888005842040 ffff888005840d08 ffff888005840d08
  head: ffff888008f51400 00000000000a0003 00000000f5000000 0000000000000000
  head: 4000000000000002 ffffea000023d401 00000000ffffffff 00000000ffffffff
  head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
  page dumped because: kasan: bad access detected

  Memory state around the buggy address:
   ffff888008f50180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
   ffff888008f50200: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  >ffff888008f50280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                ^
   ffff888008f50300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
   ffff888008f50380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
  ==================================================================

There is a second race while creating a listener. listen() starts
accepting connections before the endpoint is linked to the CM ID and
changed from IDLE to LISTENING. ACCEPT work queued in this interval can
run immediately, see the IDLE state, and return without accepting the
first child.

Keep the listener endpoint in use from allocation until its CM ID, listener
list entry, and LISTENING state have all been published. This makes early
ACCEPT work wait for listener setup to complete.

During accept, serialize the endpoint handoff with socket state changes and
reject a child that has already reached TCP_CLOSE or TCP_CLOSE_WAIT. Ignore
close notifications inherited from the listener before that handoff. Live
accepted sockets continue through the normal close path.

The issue was found by static analysis.

The unpatched kernel reproduced the KASAN report above in an x86_64 QEMU
guest. With a temporary 500 ms delay after listen(), the previous version
lost the first child and timed out waiting for an MPA response; this
version accepted the same child and returned an MPA response.
RST-before-accept and MPA-request-followed-by-FIN tests also completed
without KASAN or lockdep reports. The final SIW object was built with
LLVM 22.1.8 and W=1.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Suggested-by: Stefan Metzmacher <metze@samba.org>
Link: https://lore.kernel.org/r/33abff2233b36a51e468d691cb4327d0294d2734.1620343860.git.metze@samba.org
Cc: stable@vger.kernel.org
Signed-off-by: Jaeyeong Lee <lee@jaeyeong.cc>
---
Changes in v3:
- Serialize listener setup so early ACCEPT work cannot be consumed in IDLE.
- Include the complete KASAN report requested by Leon.

Changes in v2:
- Cover the IDLE interval between listen() and the LISTENING transition.
- Serialize accepted-socket handoff and reject an already closed child.

 drivers/infiniband/sw/siw/siw_cm.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index ed49818793dde..90adb9543d9d0 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -973,6 +973,7 @@ static void siw_accept_newconn(struct siw_cep *cep)
 	struct socket *s = cep->sock;
 	struct socket *new_s = NULL;
 	struct siw_cep *new_cep = NULL;
+	bool peer_closed;
 	int rv = 0; /* debug only. should disappear */
 
 	if (cep->state != SIW_EPSTATE_LISTENING)
@@ -1009,11 +1010,23 @@ static void siw_accept_newconn(struct siw_cep *cep)
 	}
 	new_cep->sock = new_s;
 	siw_cep_get(new_cep);
-	new_s->sk->sk_user_data = new_cep;
 
 	if (siw_tcp_nagle == false)
 		tcp_sock_set_nodelay(new_s->sk);
+
+	lock_sock(new_s->sk);
+	write_lock_bh(&new_s->sk->sk_callback_lock);
 	new_cep->state = SIW_EPSTATE_AWAIT_MPAREQ;
+	new_s->sk->sk_user_data = new_cep;
+	peer_closed = new_s->sk->sk_state == TCP_CLOSE ||
+		      new_s->sk->sk_state == TCP_CLOSE_WAIT;
+	write_unlock_bh(&new_s->sk->sk_callback_lock);
+	release_sock(new_s->sk);
+
+	if (peer_closed) {
+		rv = -ECONNRESET;
+		goto error;
+	}
 
 	rv = siw_cm_queue_work(new_cep, SIW_CM_WORK_MPATIMEOUT);
 	if (rv)
@@ -1361,6 +1374,10 @@ static void siw_cm_llp_state_change(struct sock *sk)
 
 	case TCP_CLOSE:
 	case TCP_CLOSE_WAIT:
+		if (cep->state == SIW_EPSTATE_LISTENING ||
+		    (cep->state == SIW_EPSTATE_IDLE &&
+		     !READ_ONCE(sk->sk_socket)))
+			break;
 		if (cep->qp)
 			cep->qp->tx_ctx.tx_suspend = 1;
 		siw_cm_queue_work(cep, SIW_CM_WORK_PEER_CLOSE);
@@ -1880,6 +1897,7 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 		rv = -ENOMEM;
 		goto error;
 	}
+	siw_cep_set_inuse(cep);
 	siw_cep_socket_assoc(cep, s);
 
 	rv = siw_cm_alloc_work(cep, backlog);
@@ -1932,14 +1950,13 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
 
 	siw_dbg(id->device, "Listen at laddr %pISp\n", &id->local_addr);
 
+	siw_cep_set_free(cep);
 	return 0;
 
 error:
 	siw_dbg(id->device, "failed: %d\n", rv);
 
 	if (cep) {
-		siw_cep_set_inuse(cep);
-
 		siw_free_cm_id(cep);
 		siw_socket_disassoc(s);
 		cep->state = SIW_EPSTATE_CLOSED;
-- 
2.43.0

Re: [PATCH v3] RDMA/siw: Fix early peer close handling on listeners

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/siw: premature listener CEP release before sock_release() on error
--

commit 9e6e7c6fb2387746734048153461994802a461df
Author: Jaeyeong Lee <lee@jaeyeong.cc>

RDMA/siw: Fix early peer close handling on listeners

This commit fixes a use-after-free in siw_destroy_listen() and a race during
listener creation by ignoring early peer close events on listening sockets,
serializing accepted child socket handoff with socket state changes, and
holding the listener endpoint in use until setup is complete.

Link: https://lore.kernel.org/r/33abff2233b36a51e468d691cb4327d0294d2734.1620343860.git.metze@samba.org

> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
> index ed49818793dde..90adb9543d9d0 100644
> --- a/drivers/infiniband/sw/siw/siw_cm.c
> +++ b/drivers/infiniband/sw/siw/siw_cm.c

[ ... ]

> @@ -1361,6 +1374,10 @@ static void siw_cm_llp_state_change(struct sock *sk)
>  
>  	case TCP_CLOSE:
>  	case TCP_CLOSE_WAIT:
> +		if (cep->state == SIW_EPSTATE_LISTENING ||
> +		    (cep->state == SIW_EPSTATE_IDLE &&
> +		     !READ_ONCE(sk->sk_socket)))
> +			break;
>  		if (cep->qp)
>  			cep->qp->tx_ctx.tx_suspend = 1;
>  		siw_cm_queue_work(cep, SIW_CM_WORK_PEER_CLOSE);
> @@ -1880,6 +1897,7 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
>  		rv = -ENOMEM;
>  		goto error;
>  	}
> +	siw_cep_set_inuse(cep);
>  	siw_cep_socket_assoc(cep, s);
>  
>  	rv = siw_cm_alloc_work(cep, backlog);
> @@ -1932,14 +1950,13 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
>  
>  	siw_dbg(id->device, "Listen at laddr %pISp\n", &id->local_addr);
>  
> +	siw_cep_set_free(cep);
>  	return 0;
>  
>  error:
>  	siw_dbg(id->device, "failed: %d\n", rv);
>  
>  	if (cep) {
> -		siw_cep_set_inuse(cep);
> -
>  		siw_free_cm_id(cep);
>  		siw_socket_disassoc(s);
>  		cep->state = SIW_EPSTATE_CLOSED;

[Severity: High]
Can the error cleanup path in siw_create_listen() trigger a use-after-free on
cep if id->provider_data allocation fails after s->ops->listen() has started?

In siw_create_listen(), s->ops->listen(s, backlog) places s->sk into
TCP_LISTEN before id->provider_data is allocated:

	rv = s->ops->listen(s, backlog);
	if (rv) {
		siw_dbg(id->device, "listen error %d\n", rv);
		goto error;
	}
	cep->cm_id = id;
	id->add_ref(id);
	...
	if (!id->provider_data) {
		id->provider_data =
			kmalloc_obj(struct list_head);
		if (!id->provider_data) {
			rv = -ENOMEM;
			goto error;
		}

During this window, an incoming connection can complete the TCP handshake in
softirq context, creating a child socket in the accept queue that inherits
sk_user_data = cep and the siw_cm_llp_* callbacks, and queueing
SIW_CM_WORK_ACCEPT on cep.

If kmalloc_obj() then fails and jumps to error, siw_socket_disassoc(s) only
clears sk_user_data and restores callbacks on the parent listening socket
s->sk, leaving the unaccepted child socket with sk_user_data = cep:

error:
	if (cep) {
		siw_free_cm_id(cep);
		siw_socket_disassoc(s);
		cep->state = SIW_EPSTATE_CLOSED;

		siw_cep_set_free_and_put(cep);
	}
	sock_release(s);

Because cep->state = SIW_EPSTATE_CLOSED and siw_cep_set_free_and_put(cep) run
before sock_release(s) (unlike in siw_drop_listeners(), where sock_release(s)
is called while cep->in_use is held and before cep->state is set to
SIW_EPSTATE_CLOSED), two problems can occur before sock_release(s) finishes
tearing down the unaccepted child socket:

1. Once siw_cep_set_free_and_put(cep) releases cep->in_use and drops the
listener reference, the pending SIW_CM_WORK_ACCEPT work item on siw_cm_wq
wakes up, sees cep->state != SIW_EPSTATE_LISTENING in siw_accept_newconn(),
and drops the last reference on cep, freeing it. When sock_release(s) then
runs:

  sock_release(s)
    -> inet_release()
      -> tcp_close()
        -> inet_csk_listen_stop()
          -> inet_child_forget()
            -> tcp_disconnect()
              -> siw_cm_llp_error_report(child)

siw_cm_llp_error_report() dereferences the freed cep via sk_to_cep(child).

2. If the peer closes or resets the unaccepted child after cep->state is set
to SIW_EPSTATE_CLOSED, siw_cm_llp_state_change(child) only ignores
SIW_EPSTATE_LISTENING and SIW_EPSTATE_IDLE, so it will proceed to queue
SIW_CM_WORK_PEER_CLOSE on cep (or on already-freed memory).

Should siw_create_listen() call sock_release(s) before setting cep->state =
SIW_EPSTATE_CLOSED and calling siw_cep_set_free_and_put(cep) (matching
siw_drop_listeners()), or allocate id->provider_data before calling
s->ops->listen()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105022253.368316-1-lee@jaeyeong.cc?part=1