[PATCH v2] net: qrtr: fix refcount saturation and potential UAF in qrtr_port_remove

[PATCH v2] net: qrtr: fix refcount saturation and potential UAF in qrtr_port_remove

[w15303746062]

From: Mingyu Wang <25181214217@stu.xidian.edu.cn>

In qrtr_port_remove(), the socket reference count is decremented via
__sock_put() before the port is removed from the qrtr_ports XArray and
before the RCU grace period elapses.

This breaks the fundamental RCU update paradigm. It exposes a race
window where a concurrent RCU reader (such as qrtr_reset_ports() or
qrtr_port_lookup()) can obtain a pointer to the socket from the XArray,
and attempt to call sock_hold() on a socket whose reference count has
already dropped to zero.

This exact race condition was hit during syzkaller fuzzing, leading to
the following refcount saturation warning and a potential Use-After-Free:

  refcount_t: saturated; leaking memory.
  WARNING: CPU: 3 PID: 1273 at lib/refcount.c:22 refcount_warn_saturate+0xae/0x1d0
  Modules linked in: qrtr(+) bochs drm_shmem_helper ...
  Call Trace:
   <TASK>
   qrtr_reset_ports net/qrtr/af_qrtr.c:768 [inline] [qrtr]
   __qrtr_bind.isra.0+0x48b/0x570 net/qrtr/af_qrtr.c:805 [qrtr]
   qrtr_bind+0x17d/0x210 net/qrtr/af_qrtr.c:901 [qrtr]
   kernel_bind+0xe4/0x120 net/socket.c:3592
   qrtr_ns_init+0x1a6/0x380 net/qrtr/ns.c:715 [qrtr]
   qrtr_proto_init+0x3b/0xff0 net/qrtr/af_qrtr.c:169 [qrtr]
   do_one_initcall+0xf5/0x5e0 init/main.c:1283
   ...
   </TASK>

Fix this by deferring the reference count decrement until after the
xa_erase() and the synchronize_rcu() complete.

(Note: The v1 of this patch incorrectly replaced __sock_put() with
sock_put(). As Simon Horman pointed out, the callers of qrtr_port_remove()
still hold a reference to the socket, so freeing the socket memory here
would lead to a subsequent UAF in the caller. Thus, the __sock_put() is
kept, but only repositioned to close the RCU race.)

Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
---
v1: https://lore.kernel.org/netdev/20260530082243.1123402-1-w15303746062@163.com/

Changes in v2:
- Reverted the change from __sock_put() to sock_put() to avoid UAF
  in callers like qrtr_release(), as suggested by Simon Horman.
- Removed misleading "memory leak" wording from commit message.
- Corrected the Fixes tag to point to the initial commit introducing
  the race (bdabad3e363d).

 net/qrtr/af_qrtr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 7cec6a7859b0..db823177e636 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -707,13 +707,13 @@ static void qrtr_port_remove(struct qrtr_sock *ipc)
 	if (port == QRTR_PORT_CTRL)
 		port = 0;
 
-	__sock_put(&ipc->sk);
-
 	xa_erase(&qrtr_ports, port);
 
 	/* Ensure that if qrtr_port_lookup() did enter the RCU read section we
 	 * wait for it to up increment the refcount */
 	synchronize_rcu();
+
+	__sock_put(&ipc->sk);
 }
 
 /* Assign port number to socket.
-- 
2.34.1

Re: [PATCH v2] net: qrtr: fix refcount saturation and potential UAF in qrtr_port_remove

[Simon Horman]

On Thu, Jun 04, 2026 at 02:48:01PM +0800, w15303746062@163.com wrote:
> From: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> 
> In qrtr_port_remove(), the socket reference count is decremented via
> __sock_put() before the port is removed from the qrtr_ports XArray and
> before the RCU grace period elapses.
> 
> This breaks the fundamental RCU update paradigm. It exposes a race
> window where a concurrent RCU reader (such as qrtr_reset_ports() or
> qrtr_port_lookup()) can obtain a pointer to the socket from the XArray,
> and attempt to call sock_hold() on a socket whose reference count has
> already dropped to zero.
> 
> This exact race condition was hit during syzkaller fuzzing, leading to
> the following refcount saturation warning and a potential Use-After-Free:
> 
>   refcount_t: saturated; leaking memory.
>   WARNING: CPU: 3 PID: 1273 at lib/refcount.c:22 refcount_warn_saturate+0xae/0x1d0
>   Modules linked in: qrtr(+) bochs drm_shmem_helper ...
>   Call Trace:
>    <TASK>
>    qrtr_reset_ports net/qrtr/af_qrtr.c:768 [inline] [qrtr]
>    __qrtr_bind.isra.0+0x48b/0x570 net/qrtr/af_qrtr.c:805 [qrtr]
>    qrtr_bind+0x17d/0x210 net/qrtr/af_qrtr.c:901 [qrtr]
>    kernel_bind+0xe4/0x120 net/socket.c:3592
>    qrtr_ns_init+0x1a6/0x380 net/qrtr/ns.c:715 [qrtr]
>    qrtr_proto_init+0x3b/0xff0 net/qrtr/af_qrtr.c:169 [qrtr]
>    do_one_initcall+0xf5/0x5e0 init/main.c:1283
>    ...
>    </TASK>
> 
> Fix this by deferring the reference count decrement until after the
> xa_erase() and the synchronize_rcu() complete.
> 
> (Note: The v1 of this patch incorrectly replaced __sock_put() with
> sock_put(). As Simon Horman pointed out, the callers of qrtr_port_remove()
> still hold a reference to the socket, so freeing the socket memory here
> would lead to a subsequent UAF in the caller. Thus, the __sock_put() is
> kept, but only repositioned to close the RCU race.)
> 
> Fixes: bdabad3e363d ("net: Add Qualcomm IPC router")
> Signed-off-by: Mingyu Wang <25181214217@stu.xidian.edu.cn>
> ---
> v1: https://lore.kernel.org/netdev/20260530082243.1123402-1-w15303746062@163.com/
> 
> Changes in v2:
> - Reverted the change from __sock_put() to sock_put() to avoid UAF
>   in callers like qrtr_release(), as suggested by Simon Horman.
> - Removed misleading "memory leak" wording from commit message.
> - Corrected the Fixes tag to point to the initial commit introducing
>   the race (bdabad3e363d).

Thanks for the updates.

Reviewed-by: Simon Horman <horms@kernel.org>