[PATCH net] llc: stop connection timers before dropping sap ref on release

["Miguel García Román" <miguelgarciaroman8@gmail.com>]

syzbot reported a KASAN slab-use-after-free in
llc_conn_ac_send_sabme_cmd_p_set_x() when a connection timer fires
after the associated llc_sap has already been freed via RCU.

llc_ui_release() currently does:

  llc_sap_hold(sap);
  llc_sap_remove_socket(...);   /* may drop last socket's sap ref */
  release_sock(sk);
  llc_sap_put(sap);             /* last ref -> llc_sap_close -> kfree_rcu */
  ...
  llc_sk_free(sk);              /* timer_delete_sync() only here */

Connection timers (ack / P / REJ / busy) are embedded in llc_sock and
their callbacks run llc_process_tmr_ev() -> state machine actions that
dereference llc->sap (e.g. sap->laddr.lsap). Those timers are only
synchronously cancelled in llc_sk_free(), which runs *after* the sap
reference is dropped. If this was the last socket on the sap, the sap
can be RCU-freed while a timer is still pending or running, and the
softirq timer path UAFs the freed sap.

llc_ui_release() already keeps an extra sap reference across
release_sock() so backlog processing can still use the sap. Extend that
window: after release_sock(), synchronously stop all connection timers
while the sap is still held, then drop the sap reference.
timer_delete_sync() must not run under lock_sock() (timer callbacks take
bh_lock_sock()), so stopping after release_sock() is required.

llc_sk_free() still stops timers again; a second sync delete is
harmless.

Fixes: f7e43672683b ("llc: hold llc_sap before release_sock()")
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>
---
 net/llc/af_llc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index b0447c33d..4fc397fad 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -215,6 +215,14 @@ static int llc_ui_release(struct socket *sock)
 		llc_sap_hold(sap);
 		llc_sap_remove_socket(llc->sap, sk);
 		release_sock(sk);
+		/*
+		 * Timers dereference llc->sap.  Cancel them while the sap is
+		 * still held; llc_sk_free() runs after the final sap put and
+		 * would otherwise race with kfree_rcu(sap).  Must run after
+		 * release_sock() to avoid deadlock with bh_lock_sock() in the
+		 * timer callbacks.
+		 */
+		llc_sk_stop_all_timers(sk, true);
 		llc_sap_put(sap);
 	} else {
 		release_sock(sk);
-- 
2.43.0