* [PATCH rdma-rc] RDMA/odp: Ensure the mm is still alive before creating an implicit child
@ 2020-02-27 11:41 Leon Romanovsky
2020-03-04 18:04 ` Jason Gunthorpe
0 siblings, 1 reply; 2+ messages in thread
From: Leon Romanovsky @ 2020-02-27 11:41 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe; +Cc: linux-rdma
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);
- 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);
--
2.24.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH rdma-rc] RDMA/odp: Ensure the mm is still alive before creating an implicit child
2020-02-27 11:41 [PATCH rdma-rc] RDMA/odp: Ensure the mm is still alive before creating an implicit child Leon Romanovsky
@ 2020-03-04 18:04 ` Jason Gunthorpe
0 siblings, 0 replies; 2+ messages in thread
From: Jason Gunthorpe @ 2020-03-04 18:04 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, linux-rdma
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);
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-03-04 18:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-27 11:41 [PATCH rdma-rc] RDMA/odp: Ensure the mm is still alive before creating an implicit child Leon Romanovsky
2020-03-04 18:04 ` Jason Gunthorpe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.