Re: fasync_remove_entry oops

[Dave Jones <davej@redhat.com>]

On Thu, Mar 07, 2013 at 03:46:24PM -0800, Linus Torvalds wrote:
 > On Thu, Mar 7, 2013 at 3:14 PM, Dave Jones <davej@redhat.com> wrote:
 > > And.. More fun with pipes.
 > >         for (fp = fapp; (fa = *fp) != NULL; fp = &fa->fa_next) {
 > >     1650:       49 8b 06                mov    (%r14),%rax
 > >
 > > So we got to fasync_remove_entry with a NULL fa struct.
 > >
 > > Can we just add more NULL checks here, or does that need to happen
 > > at a higher level ?
 > 
 > I think you'll find that it's not fapp that was NULL.

yeah, brainfart

 > The caller was
 > pipe_rdwr_fasync -> fasync_helper, and pipe_rdwr_fasync always passes
 > in
 > 
 >     &pipe->fasync_readers
 > 
 > (and writers) so it looks like it is pipe that was NULL. Really odd.
 > How did the open of the pipe succeed with a NULL i_pipe? We do have
 > i_pipe == NULL, but that should happen only with a not-yet-opened
 > pipe, or after the last close.
 > 
 > In neither case should you have that pipe_rdwr_fasync() call.
 > 
 > The fact that this happens for a delayed __fput() makes me think it
 > was never a successful open to begin with, but how did the FASYNC flag
 > get set in that case? Do we actually allow it in the open flags..
 > Hmm..
 > 
 > So if we need new NULL pointer checks, I think they'd need to be
 > something like the attached patch.
 
I'll give it a shot. Can't be any worse than what we have already.

 > But this is definitely another of those "This is our most desperate
 > hour. Help me, Al-biwan Ke-Viro, you're my only hope" issues.
 > 
 > Al? Please don't make me wear that golden bikini.
 
The box hosting Al's email is down, so you might be making a new fashion statement
for a little while.

	Dave