Re: [PATCH 0/3] aio: ctx->dead cleanups

[Oleg Nesterov <oleg@redhat.com>]

On 06/17, Al Viro wrote:
>
> On Wed, Jun 17, 2015 at 01:04:14AM +0200, Oleg Nesterov wrote:
> > Al, please help. We are trying to backport some aio fixes and I am
> > absolutely confused by your b2edffdd912b "fix mremap() vs. ioctx_kill()
> > race".
> >
> >
> > Firstly, I simply can't understand what exactly it tries to fix. OK,
> > aio_free_ring() can race with kill and we can remap the soon-to-be-killed
> > ctx. So what? kill_ioctx() will the the correct (already re-mapped)
> > ctx->mmap_base after it drops mm->ioctx_lock.
>
> Huh?  kill_ioctx() picks ctx->mmap_base and passes it to vm_munmap().
> Which tries to grab mmap_sem, blocks for mremap() from another thread
> and waits for it to drop mmap_sem.  By that time ctx->mmap_base has
> nothing whatsoever to the argument we'd passed to vm_munmap().

Yes. But it seems that you missed another part of my email:

	So it seems to me we only need this change to ensure that move_vma() can
	not succeed if ctx was already removed from ->ioctx_table, or, if we race
	with ioctx_alloc(), it was not added to ->ioctx_table. IOW, we need to
	ensure that move_vma()->aio_ring_mmap() can not race with
	vm_munmap(ctx->mmap_base) in kill_ioctx() or ioctx_alloc(). And this race
	doesn't look really bad. The kernel can't crash, just the application can
	fool itself.

So once again, could explain why do we really need to prevent this?
Afaics, if the application is stupid, it can only fool itself.

And please note that ctx->mmap_base or/and ctx->mmap_size can be wrong
anyway. Say, an application can munmap() this vma, or munmap() the part
of this vma.

Oleg.