Netdev List
 help / color / mirror / Atom feed
* [PATCH net v3] llc: shut down timers before dropping SAP and device references
@ 2026-10-16 13:16 Miguel García Román
  2026-10-21  2:24 ` Jakub Kicinski
  0 siblings, 1 reply; 2+ messages in thread
From: Miguel García Román @ 2026-10-16 13:16 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, horms, linux-kernel,
	syzbot+44efda9647c52be29d9c

syzbot reports a slab-use-after-free in
llc_conn_ac_send_sabme_cmd_p_set_x() when a connection timer dereferences
its SAP after the last socket reference has been dropped.

llc_ui_release() holds an extra SAP reference for release_sock(), but
drops it before llc_sk_free() stops the connection timers. If this was
the last socket using the SAP, llc_sap_close() can RCU-free it while a
timer callback is still running.

Keep the extra SAP reference until llc_sk_free() has completed. Use
timer_shutdown_sync() for final teardown: a fixed-order pass with
timer_delete_sync() permits a callback or concurrent state-machine
action to rearm an earlier timer. Keep the device reference through
timer shutdown and queue purge as well, since timer actions can use
llc->dev. The non-final timer stop path retains timer_delete().

Fixes: 9708d2b5b7c6 ("llc: avoid blocking in llc_sap_close()")
Reported-by: syzbot+44efda9647c52be29d9c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=44efda9647c52be29d9c
Signed-off-by: Miguel García Román <miguelgarciaroman8@gmail.com>

---
v3:
  - Keep SAP and device references until after final timer shutdown.
  - Use timer_shutdown_sync() to prevent cross-timer rearming.
  - Remove the separate unlocked timer stop from llc_ui_release().
  - Reproduced the KASAN report on the unpatched kernel; the patched
    kernel completed five reproducer runs without a KASAN report.
  - syzbot reproduced the UAF on the unpatched control. Two patched
    jobs built successfully, but their C reproducer aborted at bind()
    with EBUSY, so no Tested-by tag is included.
v2: https://lore.kernel.org/netdev/20260916104735.3565609-1-miguelgarciaroman8@gmail.com/
v1: https://lore.kernel.org/netdev/20260916074158.3375909-1-miguelgarciaroman8@gmail.com/

 net/llc/af_llc.c   | 11 +++++------
 net/llc/llc_conn.c |  9 +++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index b0447c33d..e34d1e7b5 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -196,6 +196,7 @@ static int llc_ui_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
 	struct llc_sock *llc;
+	struct llc_sap *sap = NULL;

 	if (unlikely(sk == NULL))
 		goto out;
@@ -207,23 +208,21 @@ static int llc_ui_release(struct socket *sock)
 	if (!llc_send_disc(sk))
 		llc_ui_wait_for_disc(sk, READ_ONCE(sk->sk_rcvtimeo));
 	if (!sock_flag(sk, SOCK_ZAPPED)) {
-		struct llc_sap *sap = llc->sap;
+		sap = llc->sap;

-		/* Hold this for release_sock(), so that llc_backlog_rcv()
-		 * could still use it.
-		 */
+		/* Keep SAP alive through backlog processing and timer shutdown. */
 		llc_sap_hold(sap);
 		llc_sap_remove_socket(llc->sap, sk);
 		release_sock(sk);
-		llc_sap_put(sap);
 	} else {
 		release_sock(sk);
 	}
-	netdev_put(llc->dev, &llc->dev_tracker);
 	sock_put(sk);
 	sock_orphan(sk);
 	sock->sk = NULL;
 	llc_sk_free(sk);
+	if (sap)
+		llc_sap_put(sap);
 out:
 	return 0;
 }
diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 260460d50..2ad5a69dd 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -942,10 +942,10 @@ void llc_sk_stop_all_timers(struct sock *sk, bool sync)
 	struct llc_sock *llc = llc_sk(sk);

 	if (sync) {
-		timer_delete_sync(&llc->pf_cycle_timer.timer);
-		timer_delete_sync(&llc->ack_timer.timer);
-		timer_delete_sync(&llc->rej_sent_timer.timer);
-		timer_delete_sync(&llc->busy_state_timer.timer);
+		timer_shutdown_sync(&llc->pf_cycle_timer.timer);
+		timer_shutdown_sync(&llc->ack_timer.timer);
+		timer_shutdown_sync(&llc->rej_sent_timer.timer);
+		timer_shutdown_sync(&llc->busy_state_timer.timer);
 	} else {
 		timer_delete(&llc->pf_cycle_timer.timer);
 		timer_delete(&llc->ack_timer.timer);
@@ -978,6 +978,7 @@ void llc_sk_free(struct sock *sk)
 	skb_queue_purge(&sk->sk_receive_queue);
 	skb_queue_purge(&sk->sk_write_queue);
 	skb_queue_purge(&llc->pdu_unack_q);
+	netdev_put(llc->dev, &llc->dev_tracker);
 #ifdef LLC_REFCNT_DEBUG
 	if (refcount_read(&sk->sk_refcnt) != 1) {
 		printk(KERN_DEBUG "Destruction of LLC sock %p delayed in %s, cnt=%d\n",
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net v3] llc: shut down timers before dropping SAP and device references
  2026-10-16 13:16 [PATCH net v3] llc: shut down timers before dropping SAP and device references Miguel García Román
@ 2026-10-21  2:24 ` Jakub Kicinski
  0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2026-10-21  2:24 UTC (permalink / raw)
  To: Miguel García Román
  Cc: netdev, davem, edumazet, pabeni, horms, linux-kernel,
	syzbot+44efda9647c52be29d9c

On Fri, 16 Oct 2026 08:16:17 -0500 Miguel García Román wrote:
> Subject: [PATCH net v3] llc: shut down timers before dropping SAP and device references

This code has been deleted.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-21  2:24 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 13:16 [PATCH net v3] llc: shut down timers before dropping SAP and device references Miguel García Román
2026-10-21  2:24 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox