linux-kernel - Re: [PATCH 2/2] aio: fix use-after-free due to missing POLLFREE handling

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]

Date:   Mon, 6 Dec 2021 11:28:13 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Benjamin LaHaise <bcrl@...ck.org>, linux-aio@...ck.org,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ramji Jiyani <ramjiyani@...gle.com>,
        Christoph Hellwig <hch@....de>,
        Oleg Nesterov <oleg@...hat.com>, Jens Axboe <axboe@...nel.dk>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH 2/2] aio: fix use-after-free due to missing POLLFREE handling

On Fri, Dec 3, 2021 at 4:23 PM Eric Biggers <ebiggers@...nel.org> wrote:
>
> require another solution.  This solution is for the queue to be cleared
> before it is freed, using 'wake_up_poll(wq, EPOLLHUP | POLLFREE);'.

Ugh.

I hate POLLFREE, and the more I look at this, the more I think it's broken.

And that

        wake_up_poll(wq, EPOLLHUP | POLLFREE);

in particular looks broken - the intent is that it should remove all
the wait queue entries (because the wait queue head is going away),
but wake_up_poll() iself actually does

        __wake_up(x, TASK_NORMAL, 1, poll_to_key(m))

where that '1' is the number of exclusive entries it will wake up.

So if there are two exclusive waiters, wake_up_poll() will simply stop
waking things up after the first one.

Which defeats the whole POLLFREE thing too.

Maybe I'm missing something, but POLLFREE really is broken.

I'd argue that all of epoll() is broken, but I guess we're stuck with it.

Now, it's very possible that nobody actually uses exclusive waits for
those wait queues, and my "nr_exclusive" argument is about something
that isn't actually a bug in reality. But I think it's a sign of
confusion, and it's just another issue with POLLFREE.

I really wish we could have some way to not have epoll and aio mess
with the wait-queue lists and cache the wait queue head pointers that
they don't own.

In the meantime, I don't think these patches make things worse, and
they may fix things. But see above about "nr_exclusive" and how I
think wait queue entries might end up avoiding POLLFREE handling..

                  Linus