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 3, 2019 at 12:30 PM Al Viro [off-list ref] wrote:

On Sun, Mar 03, 2019 at 11:44:33AM -0800, Linus Torvalds wrote:

quoted

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().

Ok, they are both confusing. The lifetime of that filp is just not
clear, with the whole "it could have been completed asynchronously"
issue.

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).

Looking more at the patch, it still looks eminently sane to me.I fixed
the silly "ki_filp" thing you noticed (I thought we made fput() take
NULL, but you're right we don't), and simplified the patch to not do
the changes that aren't necessary.

I fixed the silly "filp can be NULL" issue you pointed out (I lazily
thought we allowed fput(NULL), but you're right, we definitely don't),
and simplified the patch to not do the unnecessary changes where we
can just access the file pointer multiple different ways.

And the resulting kernel boots fine, but I doubt I have anything that
actually uses io_submit(), so that doesn't mean much.

Slightly updated patch attached anyway, even smaller than before:

 2 files changed, 36 insertions(+), 44 deletions(-)

with several of the new lines being comments about that file pointer
placement in the union.

                Linus