[PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Yang Yingliang]

Add missing mmput() on error path to avoid ref-count leak.

This problem has already been resolved in mainline by
f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").

Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/infiniband/core/umem_odp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
index eeafdc0beec7..08ef654ea9b8 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -347,7 +347,8 @@ int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem,
 		vma = find_vma(mm, ib_umem_start(umem));
 		if (!vma || !is_vm_hugetlb_page(vma)) {
 			up_read(&mm->mmap_sem);
-			return -EINVAL;
+			ret_val = -EINVAL;
+			goto out_mm;
 		}
 		h = hstate_vma(vma);
 		umem->page_shift = huge_page_shift(h);
-- 
2.25.1

Re: [PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Greg KH]

On Fri, Jun 19, 2020 at 04:03:07PM +0000, Yang Yingliang wrote:
> Add missing mmput() on error path to avoid ref-count leak.
> 
> This problem has already been resolved in mainline by
> f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").
> 
> Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/infiniband/core/umem_odp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

We need an ack by the owner/maintainer of this code, please get the IB
developers/maintainers to do that first.

Any reason why you didn't cc: them also?

thanks,

greg k-h

Re: [PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Yang Yingliang]

On 2020/7/13 22:52, Greg KH wrote:
> On Fri, Jun 19, 2020 at 04:03:07PM +0000, Yang Yingliang wrote:
>> Add missing mmput() on error path to avoid ref-count leak.
>>
>> This problem has already been resolved in mainline by
>> f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").
>>
>> Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/infiniband/core/umem_odp.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
> We need an ack by the owner/maintainer of this code, please get the IB
> developers/maintainers to do that first.

OK， I will send a new patch.

>
> Any reason why you didn't cc: them also?
>
> thanks,
>
> greg k-h
> .

Re: [PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Greg KH]

On Tue, Jul 14, 2020 at 10:57:48AM +0000, Yang Yingliang wrote:
> Add missing mmput() on error path to avoid ref-count leak.
> 
> This problem has already been resolved in mainline by
> f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").
> 
> Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/infiniband/core/umem_odp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

$ ./scripts/get_maintainer.pl --file drivers/infiniband/core/umem_odp.c
Doug Ledford <dledford@redhat.com> (supporter:INFINIBAND SUBSYSTEM)
Jason Gunthorpe <jgg@ziepe.ca> (supporter:INFINIBAND SUBSYSTEM,commit_signer:28/25=100%,authored:17/25=68%,added_lines:453/481=94%,removed_lines:662/722=92%)
Leon Romanovsky <leon@kernel.org> (commit_signer:16/25=64%)
Artemy Kovalyov <artemyko@mellanox.com> (commit_signer:4/25=16%)
Yishai Hadas <yishaih@mellanox.com> (commit_signer:3/25=12%,authored:3/25=12%)
Andrew Morton <akpm@linux-foundation.org> (commit_signer:2/25=8%)
Moni Shoua <monis@mellanox.com> (authored:2/25=8%)
linux-rdma@vger.kernel.org (open list:INFINIBAND SUBSYSTEM)
linux-kernel@vger.kernel.org (open list)

Any reason you ignored the mailing list for the whole IB developer
community?

And you need to make this REALLY obvious that this is a stable-tree-only
patch, for a specific kernel version (and why only that one version),
and a whole lot more description to allow everyone to know what is going
on, and what you expect them to review this for.

thanks,

greg k-h

Re: [PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Yang Yingliang]

On 2020/7/14 14:08, Greg KH wrote:
> On Tue, Jul 14, 2020 at 10:57:48AM +0000, Yang Yingliang wrote:
>> Add missing mmput() on error path to avoid ref-count leak.
>>
>> This problem has already been resolved in mainline by
>> f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").
>>
>> Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> ---
>>   drivers/infiniband/core/umem_odp.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
> $ ./scripts/get_maintainer.pl --file drivers/infiniband/core/umem_odp.c
> Doug Ledford <dledford@redhat.com> (supporter:INFINIBAND SUBSYSTEM)
> Jason Gunthorpe <jgg@ziepe.ca> (supporter:INFINIBAND SUBSYSTEM,commit_signer:28/25=100%,authored:17/25=68%,added_lines:453/481=94%,removed_lines:662/722=92%)
> Leon Romanovsky <leon@kernel.org> (commit_signer:16/25=64%)
> Artemy Kovalyov <artemyko@mellanox.com> (commit_signer:4/25=16%)
> Yishai Hadas <yishaih@mellanox.com> (commit_signer:3/25=12%,authored:3/25=12%)
> Andrew Morton <akpm@linux-foundation.org> (commit_signer:2/25=8%)
> Moni Shoua <monis@mellanox.com> (authored:2/25=8%)
> linux-rdma@vger.kernel.org (open list:INFINIBAND SUBSYSTEM)
> linux-kernel@vger.kernel.org (open list)
>
> Any reason you ignored the mailing list for the whole IB developer
> community?

It's my first time to send patch to stable mail list, I thought cc the 
maintainer/supporter is enough.

I will re-send to the supporter and devlep mail list.

>
> And you need to make this REALLY obvious that this is a stable-tree-only
> patch, for a specific kernel version (and why only that one version),
> and a whole lot more description to allow everyone to know what is going
> on, and what you expect them to review this for.

OK, I will describe the version in change log.


Thanks,

Yang

>
> thanks,
>
> greg k-h
> .

[PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Yang Yingliang]

Add missing mmput() on error path to avoid ref-count leak.

This problem has already been resolved in mainline by
f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").

Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
---
 drivers/infiniband/core/umem_odp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
index eeafdc0beec7..08ef654ea9b8 100644
--- a/drivers/infiniband/core/umem_odp.c
+++ b/drivers/infiniband/core/umem_odp.c
@@ -347,7 +347,8 @@ int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem,
 		vma = find_vma(mm, ib_umem_start(umem));
 		if (!vma || !is_vm_hugetlb_page(vma)) {
 			up_read(&mm->mmap_sem);
-			return -EINVAL;
+			ret_val = -EINVAL;
+			goto out_mm;
 		}
 		h = hstate_vma(vma);
 		umem->page_shift = huge_page_shift(h);
-- 
2.25.1

Re: [PATCH 4.19] IB/umem: fix reference count leak in ib_umem_odp_get()

[Jason Gunthorpe]

On Tue, Jul 14, 2020 at 10:57:48AM +0000, Yang Yingliang wrote:
> Add missing mmput() on error path to avoid ref-count leak.
> 
> This problem has already been resolved in mainline by
> f27a0d50a4bc ("RDMA/umem: Use umem->owning_mm inside ODP").
> 
> Fixes: 79bb5b7ee177 ("RDMA/umem: Fix missing mmap_sem in get umem ODP call")
> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
> ---
>  drivers/infiniband/core/umem_odp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/core/umem_odp.c b/drivers/infiniband/core/umem_odp.c
> index eeafdc0beec7..08ef654ea9b8 100644
> --- a/drivers/infiniband/core/umem_odp.c
> +++ b/drivers/infiniband/core/umem_odp.c
> @@ -347,7 +347,8 @@ int ib_umem_odp_get(struct ib_ucontext *context, struct ib_umem *umem,
>  		vma = find_vma(mm, ib_umem_start(umem));
>  		if (!vma || !is_vm_hugetlb_page(vma)) {
>  			up_read(&mm->mmap_sem);
> -			return -EINVAL;
> +			ret_val = -EINVAL;
> +			goto out_mm;
>  		}
>  		h = hstate_vma(vma);
>  		umem->page_shift = huge_page_shift(h);

This patch does look correct, please address Greg's remarks.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Thanks,
Jason