[PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[Kartik Nair]

When a timer fires while the socket is owned by a user, the timer event
is deferred to the backlog via __sk_add_backlog(). By the time the
backlog drains, llc->state may have been set to LLC_CONN_OUT_OF_SVC (0)
by socket teardown. llc_conn_state_process() then calls llc_conn_service()
which computes llc_offset_table[state - 1] = llc_offset_table[-1],
triggering UBSAN array-index-out-of-bounds.

llc_process_tmr_ev() already guards against LLC_CONN_OUT_OF_SVC for the
direct path, but this guard is bypassed when sock_owned_by_user() is true
and the event is queued to the backlog. By the time the backlog drains,
teardown may have set state to 0.

The direct path already handles this case, so the same check belongs
in the consumer too.

Reported-by: syzbot+628f93722c08dc5aabe0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=628f93722c08dc5aabe0
Signed-off-by: Kartik Nair <contact.kartikn@gmail.com>
---
 net/llc/llc_conn.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c
index 1bd6c5f56c52..1fe666b7ec1f 100644
--- a/net/llc/llc_conn.c
+++ b/net/llc/llc_conn.c
@@ -65,6 +65,11 @@ int llc_conn_state_process(struct sock *sk, struct sk_buff *skb)
 	struct llc_sock *llc = llc_sk(skb->sk);
 	struct llc_conn_state_ev *ev = llc_conn_ev(skb);
 
+	if (unlikely(llc->state == LLC_CONN_OUT_OF_SVC)) {
+		kfree_skb(skb);
+		return -ENOTCONN;
+	}
+
 	ev->ind_prim = ev->cfm_prim = 0;
 	/*
 	 * Send event to state machine
-- 
2.39.5 (Apple Git-154)

Re: [PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[Krzysztof Kozlowski]

On 15/05/2026 19:49, Kartik Nair wrote:
> When a timer fires while the socket is owned by a user, the timer event
> is deferred to the backlog via __sk_add_backlog(). By the time the
> backlog drains, llc->state may have been set to LLC_CONN_OUT_OF_SVC (0)
> by socket teardown. llc_conn_state_process() then calls llc_conn_service()
> which computes llc_offset_table[state - 1] = llc_offset_table[-1],
> triggering UBSAN array-index-out-of-bounds.
> 
> llc_process_tmr_ev() already guards against LLC_CONN_OUT_OF_SVC for the
> direct path, but this guard is bypassed when sock_owned_by_user() is true
> and the event is queued to the backlog. By the time the backlog drains,
> teardown may have set state to 0.
> 
> The direct path already handles this case, so the same check belongs
> in the consumer too.

Considering you sent similarly complex patches to MM, WiFi, accel and
now to NFC, I am sure you are using LLM without disclosing it.

Please explain with your own words what is the "direct path"?

Best regards,
Krzysztof

Re: [PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[Kartik Nair]

Sure, by "direct path," I just meant the execution flow when
the socket isn't locked by the user (!sock_owned_by_user()).

In that case, llc_process_tmr_ev() immediately calls
llc_conn_state_process(). Right before making that call, it
checks if the state is LLC_CONN_OUT_OF_SVC and bails out if
it is. That's the path that is already safe.

The issue is with the backlog path. When the socket is busy,
the skb gets queued, but by the time the backlog drains and
finally calls llc_conn_state_process(), the state might have
changed to 0 via a teardown. Since there's no re-check there,
we end up hitting llc_offset_table[-1] and triggering UBSAN.

Putting the check right at the entry of llc_conn_state_process()
fixes it for both paths.

Kartik

On Sun, May 17, 2026 at 3:30 AM Kartik Nair <contact.kartikn@gmail.com> wrote:
>
> Sure, by "direct path," I just meant the execution flow when
> the socket isn't locked by the user (!sock_owned_by_user()).
>
> In that case, llc_process_tmr_ev() immediately calls
> llc_conn_state_process(). Right before making that call, it
> checks if the state is LLC_CONN_OUT_OF_SVC and bails out if
> it is. That's the path that is already safe.
>
> The issue is with the backlog path. When the socket is busy,
> the skb gets queued, but by the time the backlog drains and
> finally calls llc_conn_state_process(), the state might have
> changed to 0 via a teardown. Since there's no re-check there,
> we end up hitting llc_offset_table[-1] and triggering UBSAN.
>
> Putting the check right at the entry of llc_conn_state_process()
> fixes it for both paths.
>
> Kartik
>
> On Sun, May 17, 2026 at 3:00 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 15/05/2026 19:49, Kartik Nair wrote:
>> > When a timer fires while the socket is owned by a user, the timer event
>> > is deferred to the backlog via __sk_add_backlog(). By the time the
>> > backlog drains, llc->state may have been set to LLC_CONN_OUT_OF_SVC (0)
>> > by socket teardown. llc_conn_state_process() then calls llc_conn_service()
>> > which computes llc_offset_table[state - 1] = llc_offset_table[-1],
>> > triggering UBSAN array-index-out-of-bounds.
>> >
>> > llc_process_tmr_ev() already guards against LLC_CONN_OUT_OF_SVC for the
>> > direct path, but this guard is bypassed when sock_owned_by_user() is true
>> > and the event is queued to the backlog. By the time the backlog drains,
>> > teardown may have set state to 0.
>> >
>> > The direct path already handles this case, so the same check belongs
>> > in the consumer too.
>>
>> Considering you sent similarly complex patches to MM, WiFi, accel and
>> now to NFC, I am sure you are using LLM without disclosing it.
>>
>> Please explain with your own words what is the "direct path"?
>>
>> Best regards,
>> Krzysztof

Re: [PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[Simon Horman]

On Fri, May 15, 2026 at 11:19:04PM +0530, Kartik Nair wrote:
> When a timer fires while the socket is owned by a user, the timer event
> is deferred to the backlog via __sk_add_backlog(). By the time the
> backlog drains, llc->state may have been set to LLC_CONN_OUT_OF_SVC (0)
> by socket teardown. llc_conn_state_process() then calls llc_conn_service()
> which computes llc_offset_table[state - 1] = llc_offset_table[-1],
> triggering UBSAN array-index-out-of-bounds.
> 
> llc_process_tmr_ev() already guards against LLC_CONN_OUT_OF_SVC for the
> direct path, but this guard is bypassed when sock_owned_by_user() is true
> and the event is queued to the backlog. By the time the backlog drains,
> teardown may have set state to 0.
> 
> The direct path already handles this case, so the same check belongs
> in the consumer too.
> 
> Reported-by: syzbot+628f93722c08dc5aabe0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=628f93722c08dc5aabe0
> Signed-off-by: Kartik Nair <contact.kartikn@gmail.com>

I notice that a similar patch was posted here:

- [PATCH net 1/1] llc: conn: drop out-of-service state in llc_conn_service
  https://lore.kernel.org/netdev/5f646c530f4a0820060499054c46b8dbecebd7be.1778638129.git.zlian064@ucr.edu/

And I wonder if it would make sense to consolidate discussion there.

Re: [PATCH] net/llc: fix UBSAN array-index-out-of-bounds in llc_conn_state_process

[Jakub Kicinski]

On Fri, 15 May 2026 23:19:04 +0530 Kartik Nair wrote:
> When a timer fires while the socket is owned by a user, the timer event
> is deferred to the backlog via __sk_add_backlog(). By the time the
> backlog drains, llc->state may have been set to LLC_CONN_OUT_OF_SVC (0)
> by socket teardown. llc_conn_state_process() then calls llc_conn_service()
> which computes llc_offset_table[state - 1] = llc_offset_table[-1],
> triggering UBSAN array-index-out-of-bounds.
> 
> llc_process_tmr_ev() already guards against LLC_CONN_OUT_OF_SVC for the
> direct path, but this guard is bypassed when sock_owned_by_user() is true
> and the event is queued to the backlog. By the time the backlog drains,
> teardown may have set state to 0.

Looks like the wrong fix, looks like Ren Wei posted a similarly wrong
fix first:
https://lore.kernel.org/all/5f646c530f4a0820060499054c46b8dbecebd7be.1778638129.git.zlian064@ucr.edu/
So I'll let them take it from here.