From: Jason Gunthorpe <jgg@nvidia.com>
To: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Cc: <dledford@redhat.com>, <linux-rdma@vger.kernel.org>,
Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>,
<stable@vger.kernel.org>, <linux-mm@kvack.org>,
Ira Weiny <ira.weiny@intel.com>
Subject: Re: [PATCH for-rc v5] IB/hfi1: Ensure correct mm is used at all times
Date: Wed, 25 Nov 2020 20:33:16 -0400 [thread overview]
Message-ID: <20201126003316.GQ4800@nvidia.com> (raw)
In-Reply-To: <20201125210112.104301.51331.stgit@awfm-01.aw.intel.com>
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
prev parent reply other threads:[~2020-11-26 0:33 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-25 21:01 Dennis Dalessandro
2020-11-26 0:33 ` Jason Gunthorpe [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201126003316.GQ4800@nvidia.com \
--to=jgg@nvidia.com \
--cc=dennis.dalessandro@cornelisnetworks.com \
--cc=dledford@redhat.com \
--cc=ira.weiny@intel.com \
--cc=linux-mm@kvack.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mike.marciniszyn@cornelisnetworks.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox