Re: [BUG] kernel BUG at fs/userfaultfd.c:385 after 04f5866e41fb

[Andrea Arcangeli <aarcange@redhat.com>]

On Wed, Aug 28, 2019 at 04:25:45PM +0200, Oleg Nesterov wrote:
> I seem to understand... Somehow I thought that __mcopy_atomic() should
> verify that dst_vma->vm_userfaultfd_ctx.ctx is the same ctx which was
> used in userfaultfd_ioctl() but it doesn't, it only checks
> dst_vma->vm_userfaultfd_ctx.ctx != NULL.
> 
> But why?
> 
> (I am just curious, let me repeat I know nothing about userfaultfd).

The ioctl fd only needs to hold an indirect reference to any ctx of
that mm. In other words the only thing the uffd ctx represents in the
UFFDIO_COPY is the destination mm, not the vma.

All the rest is done from the userland parameters of the ioctl
syscall. It's true you could have two uffd registered in the same mm
and you could call the UFFDIO_COPY ioctl on anyone of the two and it
wouldn't make any difference as long as they both are registered in
the same mm.

The uffd ctx in __mcopy_atomic is checked to be not null to be sure
the VM_MAYWRITE check run during uffd registration of the vma, before
we allow to fill any hole into the vma but it's otherwise a refcounting
neutral check.

The uffd ctx refcount in handle_userfault, that may also trigger in
copy-user invoked by the uffd ioctl -> __mcopy_atomic, works like for
any other page fault: by checking vm_userfaultfd_ctx.ctx is not null
while holding the mmap_sem with no association to the uffd ctx
refcount hold by the ioctl (again only needed to obtain the uffd
ctx->mm which is the dst_mm of the UFFDIO_COPY).

There would be nothing wrong to make it more strict, but it's not
strictly needed.

Thanks,
Andrea