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

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: <20171112175753.GB11262@redhat.com>
Date:   Sun, 12 Nov 2017 12:57:53 -0500
From:   Jerome Glisse <jglisse@...hat.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] fs: avoid NULL pointer dereference for nobh on write
 error

On Fri, Nov 10, 2017 at 12:38:29PM -0800, Linus Torvalds wrote:
> On Thu, Nov 9, 2017 at 1:49 PM,  <jglisse@...hat.com> 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