linux-kernel - Re: [PATCH v6.13] fs/netfs/read_pgpriv2: skip folio queues without `marks3`

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025021138-track-liberty-f5d9@gregkh>
Date: Tue, 11 Feb 2025 07:29:17 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Max Kellermann <max.kellermann@...os.com>
Cc: dhowells@...hat.com, netfs@...ts.linux.dev,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v6.13] fs/netfs/read_pgpriv2: skip folio queues without
 `marks3`

On Mon, Feb 10, 2025 at 11:31:44PM +0100, Max Kellermann wrote:
> At the beginning of the function, folio queues with marks3==0 are
> skipped, but after that, the `marks3` field is ignored.  If one such
> queue is found, `slot` is set to 64 (because `__ffs(0)==64`), leading
> to a buffer overflow in the folioq_folio() call.  The resulting crash
> may look like this:
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000000
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: Oops: 0000 [#1] SMP PTI
>  CPU: 11 UID: 0 PID: 2909 Comm: kworker/u262:1 Not tainted 6.13.1-cm4all2-vm #415
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.14.0-2 04/01/2014
>  Workqueue: events_unbound netfs_read_termination_worker
>  RIP: 0010:netfs_pgpriv2_write_to_the_cache+0x15a/0x3f0
>  Code: 48 85 c0 48 89 44 24 08 0f 84 24 01 00 00 48 8b 80 40 01 00 00 48 8b 7c 24 08 f3 48 0f bc c0 89 44 24 18 89 c0 48 8b 74 c7 08 <48> 8b 06 48 c7 04 24 00 10 00 00 a8 40 74 10 0f b6 4e 40 b8 00 10
>  RSP: 0018:ffffbbc440effe18 EFLAGS: 00010203
>  RAX: 0000000000000040 RBX: ffff96f8fc034000 RCX: 0000000000000000
>  RDX: 0000000000000040 RSI: 0000000000000000 RDI: ffff96f8fc036400
>  RBP: 0000000000001000 R08: ffff96f9132bb400 R09: 0000000000001000
>  R10: ffff96f8c1263c80 R11: 0000000000000003 R12: 0000000000001000
>  R13: ffff96f8fb75ade8 R14: fffffaaf5ca90000 R15: ffff96f8fb75ad00
>  FS:  0000000000000000(0000) GS:ffff9703cf0c0000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000000000 CR3: 000000010c9ca003 CR4: 00000000001706b0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  Call Trace:
>   <TASK>
>   ? __die+0x1f/0x60
>   ? page_fault_oops+0x158/0x450
>   ? search_extable+0x22/0x30
>   ? netfs_pgpriv2_write_to_the_cache+0x15a/0x3f0
>   ? search_module_extables+0xe/0x40
>   ? exc_page_fault+0x62/0x120
>   ? asm_exc_page_fault+0x22/0x30
>   ? netfs_pgpriv2_write_to_the_cache+0x15a/0x3f0
>   ? netfs_pgpriv2_write_to_the_cache+0xf6/0x3f0
>   netfs_read_termination_worker+0x1f/0x60
>   process_one_work+0x138/0x2d0
>   worker_thread+0x2a5/0x3b0
>   ? __pfx_worker_thread+0x10/0x10
>   kthread+0xba/0xe0
>   ? __pfx_kthread+0x10/0x10
>   ret_from_fork+0x30/0x50
>   ? __pfx_kthread+0x10/0x10
>   ret_from_fork_asm+0x1a/0x30
>   </TASK>
> 
> Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")
> Cc: stable@...r.kernel.org
> Signed-off-by: Max Kellermann <max.kellermann@...os.com>
> ---
> Note this patch doesn't apply to v6.14 as it was obsoleted by commit
> e2d46f2ec332 ("netfs: Change the read result collector to only use one
> work item").

Why can't we just take what is upstream instead?

Diverging from that ALWAYS ends up being more work and problems in the
end.  Only do so if you have no other choice.

thanks,

greg k-h