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  PHC 
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sun, 3 Mar 2019 20:30:11 +0000
From:   Al Viro <>
To:     Linus Torvalds <>
Cc:     Eric Dumazet <>,
        David Miller <>,
        Jason Baron <>,,,,
        Linux List Kernel Mailing <>,
        Netdev <>,,,,
        Christoph Hellwig <>
Subject: Re: [PATCH] aio: prevent the final fput() in the middle of
 vfs_poll() (Re: KASAN: use-after-free Read in unix_dgram_poll)

On Sun, Mar 03, 2019 at 11:44:33AM -0800, Linus Torvalds wrote:
> On Sun, Mar 3, 2019 at 7:18 AM Al Viro <> wrote:
> >
> > > Maybe unrelated to this bug, but...  What's to prevent a wakeup
> > > that happens just after we'd been added to a waitqueue by ->poll()
> > > triggering aio_poll_wake(), which gets to aio_poll_complete()
> > > with its fput() *before* we'd reached the end of ->poll() instance?
> I'm assuming you're talking about the second vfs_poll() in
> aio_poll_complete_work()? The one we call before we check for
> "rew->cancelled" properly under the spinlock?

No.  The first one, right in aio_poll().

> But as far as I can tell, they *all* could do:
>  - look up file in aio_submit() when allocating and creating the aio_kiocb
>  - drop the filp in 'iocb_put()' (which happens whether things
> complete successfully or not).
> and we'd have avoided a lot of complexity, and we'd have avoided this bug.
> Your patch fixes the poll() case, but it does so by just letting the
> existing complexity remain, and adding a second fget/fput pair in the
> poll logic.
> It would seem like it would be much better to rip all the complexity
> out entirely, and replace it with sane, simple and obviously correct
> code.
> Hmm?
> In other words, why wouldn't something like the attached work instead?
I'll need to dig out the mail archive from last year, but IIRC this
had been considered and there'd been non-trivial problems.  Give me
an hour or so and I'll dig that out (there'd been many rounds of
review, with long threads involved, some private, some on fsdevel).

> @@ -1060,6 +1071,7 @@ static inline void iocb_put(struct aio_kiocb *iocb)
>  {
>  	if (refcount_read(&iocb->ki_refcnt) == 0 ||
>  	    refcount_dec_and_test(&iocb->ki_refcnt)) {
> +		fput(iocb->ki_filp);

Trivial oops here - it might be NULL.

Powered by blists - more mailing lists