[PATCH] llc: Fix NULL pointer dereference in llc_conn_state_process() when sk_socket is NULL

Jiakai Xu posted 1 patch 1 week, 6 days ago
net/llc/llc_conn.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
[PATCH] llc: Fix NULL pointer dereference in llc_conn_state_process() when sk_socket is NULL
Posted by Jiakai Xu 1 week, 6 days ago
sk->sk_socket can be NULL when a socket has been orphaned by sock_orphan()
and a pending LLC timer fires afterwards. The timer callback chain is:

  llc_conn_ack_tmr_cb() -> llc_conn_tmr_common_cb() ->
  llc_process_tmr_ev() -> llc_conn_state_process()

llc_conn_state_process() unconditionally dereferences sk->sk_socket
at four locations when handling DISC_PRIM and CONN_PRIM confirm
primitives to update the socket state (SS_UNCONNECTED / SS_CONNECTED).

Add sk->sk_socket NULL checks at all four sites so that when the
socket is gone, the state update is simply skipped rather than
triggering a kernel page fault.

Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
Signed-off-by: Jiakai Xu <xujiakai24@mails.ucas.ac.cn>
---
 net/llc/llc_conn.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 5c0ac243b248f..de65c452f6e68 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -101,7 +101,8 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
 	case LLC_DISC_PRIM:
 		sock_hold(sk);
 		if (sk->sk_type == SOCK_STREAM &&
-		    sk->sk_state == TCP_ESTABLISHED) {
+		    sk->sk_state == TCP_ESTABLISHED &&
+		    sk->sk_socket) {
 			sk->sk_shutdown       = SHUTDOWN_MASK;
 			sk->sk_socket->state  = SS_UNCONNECTED;
 			sk->sk_state          = TCP_CLOSE;
@@ -136,7 +137,8 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
 		break;
 	case LLC_CONN_PRIM:
 		if (sk->sk_type == SOCK_STREAM &&
-		    sk->sk_state == TCP_SYN_SENT) {
+		    sk->sk_state == TCP_SYN_SENT &&
+		    sk->sk_socket) {
 			if (ev->status) {
 				sk->sk_socket->state = SS_UNCONNECTED;
 				sk->sk_state         = TCP_CLOSE;
@@ -149,7 +151,7 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
 		break;
 	case LLC_DISC_PRIM:
 		sock_hold(sk);
-		if (sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_CLOSING) {
+		if (sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_CLOSING && sk->sk_socket) {
 			sk->sk_socket->state = SS_UNCONNECTED;
 			sk->sk_state         = TCP_CLOSE;
 			sk->sk_state_change(sk);
-- 
2.34.1

Found by fuzzing. Here is the report:

Unable to handle kernel paging request at virtual address dfffffff00000000
Current syz-executor pgtable: 4K pagesize, 57-bit VAs, pgdp=0x000000012bf0c000
[dfffffff00000000] pgd=000000005fffe401, p4d=000000005fffe001, pud=0000000000000000
Oops [#1]
Modules linked in:
CPU: 2 UID: 0 PID: 3127 Comm: syz-executor Tainted: G        W           7.1.0-rc1-gdb909bd7986c #1 PREEMPT 
Tainted: [W]=WARN
Hardware name: riscv-virtio,qemu (DT)
epc : llc_conn_state_process+0xcea/0x1408 net/llc/llc_conn.c:141
 ra : llc_conn_state_process+0xcdc/0x1408 net/llc/llc_conn.c:141
epc : ffffffff856171e0 ra : ffffffff856171d2 sp : ff20000000027900
 gp : ffffffff8a395420 tp : ff6000008a3e3580 t0 : ff6000008fd27a60
 t1 : ffebffff128ef628 t2 : ff600000ffa73728 s0 : ff200000000279b0
 s1 : 0000000000000000 a0 : 0000000000000001 a1 : 0000000000000000
 a2 : 0000000000f00000 a3 : ffffffff856171d2 a4 : 0000000000000000
 a5 : dfffffff00000000 a6 : 0000000000f00000 a7 : ff6000009477b143
 s2 : ff6000008c2abe00 s3 : 0000000000000002 s4 : ffffffff87a68fc0
 s5 : 0000000000000000 s6 : ff6000009477b000 s7 : 0000000000000000
 s8 : ff6000009477b000 s9 : dfffffff00000000 s10: 0000000000000000
 s11: ff6000008c2abe2d t3 : 38177e0100000000 t4 : ffebffff128ef628
 t5 : ffebffff128ef629 t6 : 0000000000000002 ssp : 0000000000000000
status: 0000000200000120 badaddr: dfffffff00000000 cause: 000000000000000d
[<ffffffff856171e0>] llc_conn_state_process+0xcea/0x1408 net/llc/llc_conn.c:141
[<ffffffff8560bd82>] llc_process_tmr_ev net/llc/llc_c_ac.c:1448 [inline]
[<ffffffff8560bd82>] llc_conn_tmr_common_cb+0x278/0x81c net/llc/llc_c_ac.c:1331
[<ffffffff856141c6>] llc_conn_ack_tmr_cb+0x1e/0x28 net/llc/llc_c_ac.c:1356
[<ffffffff8041b658>] call_timer_fn+0x208/0xcc4 kernel/time/timer.c:1748
[<ffffffff8041ca3c>] expire_timers kernel/time/timer.c:1799 [inline]
[<ffffffff8041ca3c>] __run_timers+0x928/0xe38 kernel/time/timer.c:2374
[<ffffffff8041d082>] __run_timer_base kernel/time/timer.c:2386 [inline]
[<ffffffff8041d082>] __run_timer_base kernel/time/timer.c:2378 [inline]
[<ffffffff8041d082>] run_timer_base+0x136/0x1b6 kernel/time/timer.c:2395
[<ffffffff8041d11e>] run_timer_softirq+0x1c/0x52 kernel/time/timer.c:2405
[<ffffffff80172cb2>] handle_softirqs+0x4ca/0x1564 kernel/softirq.c:622
[<ffffffff801742de>] __do_softirq kernel/softirq.c:656 [inline]
[<ffffffff801742de>] invoke_softirq kernel/softirq.c:496 [inline]
[<ffffffff801742de>] __irq_exit_rcu+0x44e/0x8cc kernel/softirq.c:735
[<ffffffff801763fc>] irq_exit_rcu+0x10/0xf8 kernel/softirq.c:752
[<ffffffff866f79a8>] handle_riscv_irq+0x40/0x4c arch/riscv/kernel/traps.c:432
[<ffffffff8672696e>] call_on_irq_stack+0x32/0x40 arch/riscv/kernel/entry.S:396
Code: faf2 80e7 c240 07b7 e000 17fd d713 0034 1782 97ba (8783) 0007 
---[ end trace 0000000000000000 ]---
----------------
Code disassembly (best guess):
   0:	faf2                	fsw	ft8,116(sp)
   2:	c24080e7          	jalr	-988(ra)
   6:	e00007b7          	lui	a5,0xe0000
   a:	17fd                	addi	a5,a5,-1 # 0xffffffffdfffffff
   c:	0034d713          	srli	a4,s1,0x3
  10:	1782                	slli	a5,a5,0x20
  12:	97ba                	add	a5,a5,a4
* 14:	00078783          	lb	a5,0(a5) <-- trapping instruction

<<<<<<<<<<<<<<< tail report >>>>>>>>>>>>>>>
Re: [PATCH] llc: Fix NULL pointer dereference in llc_conn_state_process() when sk_socket is NULL
Posted by Paolo Abeni 1 week, 4 days ago
On 5/26/26 3:35 AM, Jiakai Xu wrote:
> diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
> index 5c0ac243b248f..de65c452f6e68 100644
> --- a/net/llc/llc_conn.c
> +++ b/net/llc/llc_conn.c
> @@ -101,7 +101,8 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
>  	case LLC_DISC_PRIM:
>  		sock_hold(sk);
>  		if (sk->sk_type == SOCK_STREAM &&
> -		    sk->sk_state == TCP_ESTABLISHED) {
> +		    sk->sk_state == TCP_ESTABLISHED &&
> +		    sk->sk_socket) {
>  			sk->sk_shutdown       = SHUTDOWN_MASK;
>  			sk->sk_socket->state  = SS_UNCONNECTED;

sk orphaning happens outside the sk socket lock, and before the timer is
cancelled. sk_socket can still be cleared after the previous check and
before this access. You probably need to move the sock_orphan() call in
lc_sk_free(), after stopping the timers.

/P
Re: [PATCH] llc: Fix NULL pointer dereference in llc_conn_state_process() when sk_socket is NULL
Posted by Jiakai Xu 1 week, 3 days ago
Thanks for your review!

> > diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
> > index 5c0ac243b248f..de65c452f6e68 100644
> > --- a/net/llc/llc_conn.c
> > +++ b/net/llc/llc_conn.c
> > @@ -101,7 +101,8 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
> >  	case LLC_DISC_PRIM:
> >  		sock_hold(sk);
> >  		if (sk->sk_type == SOCK_STREAM &&
> > -		    sk->sk_state == TCP_ESTABLISHED) {
> > +		    sk->sk_state == TCP_ESTABLISHED &&
> > +		    sk->sk_socket) {
> >  			sk->sk_shutdown       = SHUTDOWN_MASK;
> >  			sk->sk_socket->state  = SS_UNCONNECTED;
> 
> sk orphaning happens outside the sk socket lock, and before the timer is
> cancelled. sk_socket can still be cleared after the previous check and
> before this access. You probably need to move the sock_orphan() call in
> lc_sk_free(), after stopping the timers.

You are right. I'll send a v2 patch later.

Regards,
Jiakai