linux-kernel - Re: fasync_remove

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFxDYcfxKO_h7ZTrs7o2+i=EjKxBtpeZWASQ75PVK7cJOA@mail.gmail.com>
Date:	Thu, 7 Mar 2013 15:46:24 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dave Jones <davej@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Al Viro <viro@...iv.linux.org.uk>
Subject: Re: fasync_remove_entry oops

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

Download attachment "patch.diff" of type "application/octet-stream" (1930 bytes)