| Message ID | 20200227114118.94736-1-leon@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | [rdma-rc] RDMA/odp: Ensure the mm is still alive before creating an implicit child | expand |
On Thu, Feb 27, 2020 at 01:41:18PM +0200, Leon Romanovsky wrote: > From: Jason Gunthorpe <jgg@mellanox.com> > > Registration of a mmu_notifier requires the caller to hold a mmget() on > the mm as registration is not permitted to race with exit_mmap(). There is > a BUG_ON inside the mmu_notifier to guard against this. > > Normally creating a umem is done against current which implicitly holds > the mmget(), however an implicit ODP child is created from a pagefault > work queue and is not guaranteed to have a mmget(). > > Call mmget() around this registration and abort faulting if the MM has > gone to exit_mmap(). > > Before the patch below the notifier was registered when the implicit ODP > parent was created, so there was no chance to register a notifier outside > of current. > > Fixes: c571feca2dc9 ("RDMA/odp: use mmu_notifier_get/put for 'struct ib_ucontext_per_mm'") > Signed-off-by: Jason Gunthorpe <jgg@mellanox.com> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com> > --- > drivers/infiniband/core/umem_odp.c | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c > index b8c657b28380..168f4f260c23 100644 > --- a/drivers/infiniband/core/umem_odp.c > +++ b/drivers/infiniband/core/umem_odp.c > @@ -181,14 +181,27 @@ ib_umem_odp_alloc_child(struct ib_umem_odp *root, unsigned long addr, > odp_data->page_shift = PAGE_SHIFT; > odp_data->notifier.ops = ops; > > + /* > + * A mmget must be held when registering a notifier, the owming_mm only > + * has a mm_grab at this point. > + */ > + if (!mmget_not_zero(umem->owning_mm)) { > + ret = -EFAULT; > + goto out_free; > + } > + > odp_data->tgid = get_pid(root->tgid); > ret = ib_init_umem_odp(odp_data, ops); > - if (ret) { > - put_pid(odp_data->tgid); This put_pid got lost, I put it back before applying to for-rc: diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c index b8c657b2838048..cd656ad4953bfc 100644 --- a/drivers/infiniband/core/umem_odp.c +++ b/drivers/infiniband/core/umem_odp.c @@ -181,14 +181,28 @@ ib_umem_odp_alloc_child(struct ib_umem_odp *root, unsigned long addr, odp_data->page_shift = PAGE_SHIFT; odp_data->notifier.ops = ops; + /* + * A mmget must be held when registering a notifier, the owming_mm only + * has a mm_grab at this point. + */ + if (!mmget_not_zero(umem->owning_mm)) { + ret = -EFAULT; + goto out_free; + } + odp_data->tgid = get_pid(root->tgid); ret = ib_init_umem_odp(odp_data, ops); - if (ret) { - put_pid(odp_data->tgid); - kfree(odp_data); - return ERR_PTR(ret); - } + if (ret) + goto out_tgid; + mmput(umem->owning_mm); return odp_data; + +out_tgid: + put_pid(odp_data->tgid); + mmput(umem->owning_mm); +out_free: + kfree(odp_data); + return ERR_PTR(ret); } EXPORT_SYMBOL(ib_umem_odp_alloc_child);
diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c index b8c657b28380..168f4f260c23 100644 --- a/drivers/infiniband/core/umem_odp.c +++ b/drivers/infiniband/core/umem_odp.c @@ -181,14 +181,27 @@ ib_umem_odp_alloc_child(struct ib_umem_odp *root, unsigned long addr, odp_data->page_shift = PAGE_SHIFT; odp_data->notifier.ops = ops; + /* + * A mmget must be held when registering a notifier, the owming_mm only + * has a mm_grab at this point. + */ + if (!mmget_not_zero(umem->owning_mm)) { + ret = -EFAULT; + goto out_free; + } + odp_data->tgid = get_pid(root->tgid); ret = ib_init_umem_odp(odp_data, ops); - if (ret) { - put_pid(odp_data->tgid); - kfree(odp_data); - return ERR_PTR(ret); - } + if (ret) + goto out_mm; + mmput(umem->owning_mm); return odp_data; + +out_mm: + mmput(umem->owning_mm); +out_free: + kfree(odp_data); + return ERR_PTR(ret); } EXPORT_SYMBOL(ib_umem_odp_alloc_child);