linux-kernel - Re: [PATCH] mm/filemap: fix NULL pointer dereference in do_read_cache

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20251119062941.GF196362@frogsfrogsfrogs>
Date: Tue, 18 Nov 2025 22:29:41 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Matthew Wilcox <willy@...radead.org>,
	Christoph Hellwig <hch@...radead.org>,
	SHAURYA RANE <ssrane_b23@...vjti.ac.in>, akpm@...ux-foundation.org,
	shakeel.butt@...ux.dev, eddyz87@...il.com, andrii@...nel.org,
	ast@...nel.org, linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, linux-kernel-mentees@...ts.linux.dev,
	skhan@...uxfoundation.org, david.hunter.linux@...il.com,
	khalid@...nel.org,
	syzbot+09b7d050e4806540153d@...kaller.appspotmail.com,
	bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH] mm/filemap: fix NULL pointer dereference in
 do_read_cache_folio()

On Tue, Nov 18, 2025 at 11:38:36AM -0800, Andrii Nakryiko wrote:
> On Tue, Nov 18, 2025 at 8:12 AM Darrick J. Wong <djwong@...nel.org> wrote:
> >
> > On Tue, Nov 18, 2025 at 03:37:09PM +0000, Matthew Wilcox wrote:
> > > On Tue, Nov 18, 2025 at 05:03:24AM -0800, Christoph Hellwig wrote:
> > > > On Mon, Nov 17, 2025 at 10:45:31AM -0800, Andrii Nakryiko wrote:
> > > > > As I replied on another email, ideally we'd have some low-level file
> > > > > reading interface where we wouldn't have to know about secretmem, or
> > > > > XFS+DAX, or whatever other unusual combination of conditions where
> > > > > exposed internal APIs like filemap_get_folio() + read_cache_folio()
> > > > > can crash.
> > > >
> > > > The problem is that you did something totally insane and it kinda works
> > > > most of the time.
> > >
> > > ... on 64-bit systems.  The HIGHMEM handling is screwed up too.
> > >
> > > > But bpf or any other file system consumer has
> > > > absolutely not business poking into the page cache to start with.
> > >
> > > Agreed.
> > >
> > > > And I'm really pissed off that you wrote and merged this code without
> > > > ever bothering to talk to a FS or MM person who have immediately told
> > > > you so.  Let's just rip out this buildid junk for now and restart
> > > > because the problem isn't actually that easy.
> > >
> > > Oh, they did talk to fs & mm people originally and were told NO, so they
> > > sneaked it in through the BPF tree.
> > >
> > > https://lore.kernel.org/all/20230316170149.4106586-1-jolsa@kernel.org/
> > >
> > > > > The only real limitation is that we'd like to be able to control
> > > > > whether we are ok sleeping or not, as this code can be called from
> > > > > pretty much anywhere BPF might run, which includes NMI context.
> > > > >
> > > > > Would this kiocb_read() approach work under those circumstances?
> > > >
> > > > No.  IOCB_NOWAIT is just a hint to avoid blocking function calls.
> > > > It is not guarantee and a guarantee is basically impossible.
> > >
> > > I'm not sure I'd go that far -- I think we're pretty good about not
> > > sleeping when IOCB_NOWAIT is specified and any remaining places can
> > > be fixed up.
> > >
> > > But I am inclined to rip out the buildid code, just because the
> > > authors have been so rude.
> >
> > Which fstest actually checks the functionality of the buildid code?
> > I don't find any, which means none of the fs people have a good signal
> > for breakage in this, um, novel file I/O path.
> 
> We have plenty of build ID tests in BPF selftest that validate this
> functionality:
> 
>   - tools/testing/selftests/bpf/prog_tests/stacktrace_build_id.c
>   - tools/testing/selftests/bpf/prog_tests/stacktrace_build_id_nmi.c
>   - tools/testing/selftests/bpf/prog_tests/build_id.c
> 
> This functionality is exposed to BPF (and PROCMAP_QUERY, which has its
> own mm selftests), so that's where we test this. So we'll know at the
> very least when trees merge that something is broken.

Only if you're testing the buildid functionality with all known file I/O
paths implemented by all filesystems.  Or you could add a new testcase
to fstests and we'd do all that *for* you.

--D

> >
> > --D
>