Re: [PATCH] mm/filemap: fix NULL pointer dereference in do_read_cache_folio()

Matthew Wilcox
2025-11-18T15:37:12Z

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.