linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jerome Glisse <jglisse@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	stable <stable@vger.kernel.org>
Subject: Re: [PATCH] fs: avoid NULL pointer dereference for nobh on write error
Date: Sun, 12 Nov 2017 12:57:53 -0500	[thread overview]
Message-ID: <20171112175753.GB11262@redhat.com> (raw)
In-Reply-To: <CA+55aFz19F6Lzsigv9CQzsF4gBDWQf5f8oBKHt4jT9zxCeTi-w@mail.gmail.com>

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

      reply	other threads:[~2017-11-12 17:57 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09 21:49 [PATCH] fs: avoid NULL pointer dereference for nobh on write error jglisse
2017-11-10 20:38 ` Linus Torvalds
2017-11-12 17:57   ` Jerome Glisse [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171112175753.GB11262@redhat.com \
    --to=jglisse@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).