linux-kernel - Re: [RESEND] Fwd: [BUG] list corruption in __bpf_lru_node_move () 【 bug found and suggestions for fixing it】

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ff1562e2-b9c7-4747-9953-75c3e8a60c99@linux.dev>
Date: Mon, 10 Mar 2025 17:46:01 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Hou Tao <houtao@...weicloud.com>, Strforexc yn <strforexc@...il.com>
Cc: "Alexei Starovoitov," <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>,
 Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
 Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>, linux-kernel@...r.kernel.org,
 bpf@...r.kernel.org
Subject: Re: [RESEND] Fwd: [BUG] list corruption in __bpf_lru_node_move () 【 bug found and suggestions for fixing it】

On 3/9/25 7:19 PM, Hou Tao wrote:
> Resend due to the HTML part in the reply. Sorry for the inconvenience.
> 
> Hi,
> 
> On 3/5/2025 9:28 PM, Strforexc yn wrote:
>> Hi Maintainers,
>>
>> When using our customized Syzkaller to fuzz the latest Linux kernel,
>> the following crash was triggered.
>> Kernel Config : https://github.com/Strforexc/LinuxKernelbug/blob/main/.config
>>
>> A kernel BUG was reported due to list corruption during BPF LRU node movement.
>> The issue occurs when the node being moved is the sole element in its list and
>> also the next_inactive_rotation candidate. After moving, the list became empty,
>> but next_inactive_rotation incorrectly pointed to the moved node, causing later
>> operations to corrupt the list.
> 
> The list being pointed by next_inactive_rotation is a doubly linked list
> (aka, struct list_head), therefore, there are at least two nodes in the
> non-empty list: the head of the list and the sole element. When the node
> is the last element in the list, next_inactive_rotation will be pointed
> to the head of the list after the move. So I don't think the analysis
> and the fix below is correct.
>>
>> Here is my fix suggestion:
>> The fix checks if the node was the only element before adjusting
>> next_inactive_rotation. If so, it sets the pointer to NULL, preventing invalid
>> access.
>>
>> diff --git a/kernel/bpf/bpf_lru_list.c b/kernel/bpf/bpf_lru_list.c
>> index XXXXXXX..XXXXXXX 100644
>> --- a/kernel/bpf/bpf_lru_list.c
>> +++ b/kernel/bpf/bpf_lru_list.c
>> @@ -119,8 +119,13 @@ static void __bpf_lru_node_move(struct bpf_lru_list *l,
>>    * move the next_inactive_rotation pointer also.
>>    */
>>    if (&node->list == l->next_inactive_rotation)
>> - l->next_inactive_rotation = l->next_inactive_rotation->prev;
>> -
>> + {
>> + if (l->next_inactive_rotation->prev == &node->list) {

I don't think it is the right fix. I don't see how both this new "if" and the 
above "if (&node->list == l->next_inactive_rotation)" can be true together. If 
it fixed the issue, the root cause should be somewhere else.

I tried to simulate a one node inactive list and then rotate to the active list. 
I cannot reproduce it.

Can you share the syzkaller reproducer that you have used to test this fix?

Is it something that you have seen recently and something that you can bisect?

>> + l->next_inactive_rotation = NULL;
>> + } else {
>> + l->next_inactive_rotation = l->next_inactive_rotation->prev;
>> + }
>> + }
>>    list_move(&node->list, &l->lists[tgt_type]);
>>   }
>>
>> -- 2.34.1 Our knowledge of the kernel is somewhat limited, and we'd
>> appreciate it if you could determine if there is such an issue. If
>> this issue doesn't have an impact, please ignore it ☺. If you fix this
>> issue, please add the following tag to the commit: Reported-by:
>> Zhizhuo Tang strforexctzzchange@...mail.com, Jianzhou Zhao
>> xnxc22xnxc22@...com, Haoran Liu <cherest_san@....com> Last is my
>> report： vmalloc memory list_add corruption. next->prev should be prev
>> (ffffe8ffac433e40), but was 50ffffe8ffac433e. (next=ffffe8ffac433e41).
>> ------------[ cut here ]------------ kernel BUG at
>> lib/list_debug.c:29! Oops: invalid opcode: 0000 [#1] PREEMPT SMP KASAN
>> PTI CPU: 0 UID: 0 PID: 14524 Comm: syz.0.285 Not tainted
>> 6.14.0-rc5-00013-g99fa936e8e4f #1 Hardware name: QEMU Standard PC
>> (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014 RIP:
>> 0010:__list_add_valid_or_report+0xfc/0x1a0 lib/list_debug.c:29
> 
> I suspect that the content of lists[BPF_LRU_LIST_T_ACTIVE].next has been
> corrupted, because the pointer itself should be at least 8-bytes
> aligned, but its value is 0xffffe8ffac433e41. Also only the last bit of

It is more puzzling. Instead of the inactive list, the active list's head is 
corrupted in the last bit of its next. I don't see the lru code path is reusing 
the last bit of the next pointer. It is not a hlist_nulls... We need the 
syzkaller reproducer to understand it better.

> the next pointer is different with the address of
> list[BPF_LRU_LIST_T_ACTIVE] itelse (aka 0xffffe8ffac433e40).