On Sun, Mar 3, 2019 at 12:30 PM Al Viro wrote: > > On Sun, Mar 03, 2019 at 11:44:33AM -0800, Linus Torvalds wrote: > > > > 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