Re: [PATCH for-rc v4] IB/hfi1: Ensure correct mm is used at all times

[Jason Gunthorpe <jgg@ziepe.ca>]

On Mon, Nov 23, 2020 at 11:50:24AM -0500, Dennis Dalessandro wrote:
> @@ -133,8 +121,16 @@ void hfi1_mmu_rb_unregister(struct mmu_rb_handler *handler)
>  	unsigned long flags;
>  	struct list_head del_list;
>  
> +	/*
> +	 * do_exit() calls exit_mm() before exit_files() which would call close
> +	 * and end up in here. If there is no mm, then its a kernel thread and
> +	 * we need to let it continue the removal.
> +	 */
> +	if (current->mm && (handler->mn.mm != current->mm))
> +		return;
> +
>  	/* Unregister first so we don't get any more notifications. */
> -	mmu_notifier_unregister(&handler->mn, handler->mm);
> +	mmu_notifier_unregister(&handler->mn, handler->mn.mm);

This logic cannot be right.. The only caller does:

		if (pq->handler)
			hfi1_mmu_rb_unregister(pq->handler);
[..]
		kfree(pq);

So this is leaking the mmu_notifier registration if the user manages
to trigger hfi1_user_sdma_free_queues() from another process.

Since hfi1_user_sdma_free_queues() is called from close() it doesn't
look OK.

When the object that creates the notifier is destroyed the notifier
should be deleted unconditionally.

Only accesses to a VA should be qualified to ensure that a notifier is
registered on current->mm before touching the VA.

Jason