Re: [PATCH] fs: avoid NULL pointer dereference for nobh on write error

From: Jerome Glisse
Date: Sun Nov 12 2017 - 12:58:02 EST


On Fri, Nov 10, 2017 at 12:38:29PM -0800, Linus Torvalds wrote:
> On Thu, Nov 9, 2017 at 1:49 PM, <jglisse@xxxxxxxxxx> wrote:
> >
> > (Result of code inspection only, i do not have a bug, nor know a bug
> > that would be explain by this issue. Is there a kernel trace database
> > one can query for that ?)
>
> This is intentional.
>
> See the comment above the code you added:
>
> * Be careful: the buffer linked list is a NULL terminated one, rather
> * than the circular one we're used to.
>
> and then nobh_write_end() does:
>
> struct buffer_head *head = fsdata;
> ...
> while (head) {
> bh = head;
> head = head->b_this_page;
> free_buffer_head(bh);
> }
>
> so it *depends* on the bh list being NULL-terminated.
>
> So your patch is definitely wrong and breaks that nobh_write_end() case.
>
> Which is not to say that there couldn't be a NULL pointer dereference
> in some error path exactly because this code intentionally breaks the
> normal rules.
>
> But no, I'm definitely not applying this patch as-is, and not just before 4.14.

You are right, i will rework that as part of my patchset, this is all 4.16
material at best anyway. If i get sometime i will try to trigger the issue
on nobh this week and send a fix that only make the list circular inside
attach_nobh_buffers() which is were the SEGFAULT would happen and where we
would need to make it circular before attaching to the page as other code
in the kernel expect that list to be circular.

Jérôme