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
