On Wed, Nov 25, 2020 at 04:01:12PM -0500, Dennis Dalessandro wrote:
Two earlier bug fixes have created a security problem in the hfi1 driver. One fix aimed to solve an issue where current->mm was not valid when closing the hfi1 cdev. It attempted to do this by saving a cached value of the current->mm pointer at file open time. This is a problem if another process with access to the FD calls in via write() or ioctl() to pin pages via the hfi driver. The other fix tried to solve a use after free by taking a reference on the mm.
To fix this correctly we use the existing cached value of the mm in the mmu notifier. Now we can check in the insert, evict, etc. routines that current->mm matched what the notifier was registered for. If not, then don't allow access. The register of the mmu notifier will save the mm pointer.
Note the check in the unregister is not needed in the event that current->mm is empty. This means the tear down is happening due to a SigKill or OOM Killer, something along those lines. If current->mm has a value then it must be checked and only the task that did the register can do the unregister.
I deleted this paragraph, it doesn't apply any more
Since in do_exit() the exit_mm() is called before exit_files(), which would call our close routine a reference is needed on the mm. We rely on the mmgrab done by the registration of the notifier, whereas before it was explicit. The mmu notifier deregistration happens when the user context is torn down, the creation of which triggered the registration.
Also of note is we do not do any explicit work to protect the interval tree notifier. It doesn't seem that this is going to be needed since we aren't actually doing anything with current->mm. The interval tree notifier stuff still has a FIXME noted from a previous commit that will be addressed in a follow on patch.
Fixes: e0cf75deab81 ("IB/hfi1: Fix mm_struct use after free") Fixes: 3faa3d9a308e ("IB/hfi1: Make use of mm consistent") Cc: stable@vger.kernel.org Suggested-by: Jann Horn jannh@google.com Reported-by: Jason Gunthorpe jgg@nvidia.com Reviewed-by: Ira Weiny ira.weiny@intel.com Reviewed-by: Mike Marciniszyn mike.marciniszyn@cornelisnetworks.com Signed-off-by: Dennis Dalessandro dennis.dalessandro@cornelisnetworks.com
Applied to for-rc
Thanks, Jason