Re: fasync_remove_entry oops

[Linus Torvalds]

On Thu, Mar 7, 2013 at 3:14 PM, Dave Jones <davej@xxxxxxxxxx> 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. 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.

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.

            Linus

Attachment: patch.diff
Description: Binary data