All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zhu Yanjun <zyjzyj2000@gmail.com>
To: Bob Pearson <rpearsonhpe@gmail.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>,
	RDMA mailing list <linux-rdma@vger.kernel.org>,
	haakon.brugge@oracle.com
Subject: Re: [PATCH for-next] RDMA/rxe: Fix memory leak in error path code
Date: Mon, 5 Jul 2021 11:42:21 +0800	[thread overview]
Message-ID: <CAD=hENeHRjL8YhjwWi-dnknFAJeDUyHK3s-TdQf2AF853MHCMw@mail.gmail.com> (raw)
In-Reply-To: <20210704223506.12795-1-rpearsonhpe@gmail.com>

On Mon, Jul 5, 2021 at 6:37 AM Bob Pearson <rpearsonhpe@gmail.com> wrote:
>
> In rxe_mr_init_user() in rxe_mr.c at the third error the driver fails to
> free the memory at mr->map. This patch adds code to do that.
> This error only occurs if page_address() fails to return a non zero address
> which should never happen for 64 bit architectures.

If this will never happen for 64 bit architectures, is it possible to
exclude 64 bit architecture with some MACROs or others?

Thanks,

Zhu Yanjun

>
> Fixes: 8700e3e7c485 ("Soft RoCE driver")
> Reported by: Haakon Bugge <haakon.bugge@oracle.com>
> Signed-off-by: Bob Pearson <rpearsonhpe@gmail.com>
> ---
>  drivers/infiniband/sw/rxe/rxe_mr.c | 41 +++++++++++++++++-------------
>  1 file changed, 24 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c
> index 6aabcb4de235..f49baff9ca3d 100644
> --- a/drivers/infiniband/sw/rxe/rxe_mr.c
> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c
> @@ -106,20 +106,21 @@ void rxe_mr_init_dma(struct rxe_pd *pd, int access, struct rxe_mr *mr)
>  int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
>                      int access, struct rxe_mr *mr)
>  {
> -       struct rxe_map          **map;
> -       struct rxe_phys_buf     *buf = NULL;
> -       struct ib_umem          *umem;
> -       struct sg_page_iter     sg_iter;
> -       int                     num_buf;
> -       void                    *vaddr;
> +       struct rxe_map **map;
> +       struct rxe_phys_buf *buf = NULL;
> +       struct ib_umem *umem;
> +       struct sg_page_iter sg_iter;
> +       int num_buf;
> +       void *vaddr;
>         int err;
> +       int i;
>
>         umem = ib_umem_get(pd->ibpd.device, start, length, access);
>         if (IS_ERR(umem)) {
> -               pr_warn("err %d from rxe_umem_get\n",
> -                       (int)PTR_ERR(umem));
> +               pr_warn("%s: Unable to pin memory region err = %d\n",
> +                       __func__, (int)PTR_ERR(umem));
>                 err = PTR_ERR(umem);
> -               goto err1;
> +               goto err_out;
>         }
>
>         mr->umem = umem;
> @@ -129,15 +130,15 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
>
>         err = rxe_mr_alloc(mr, num_buf);
>         if (err) {
> -               pr_warn("err %d from rxe_mr_alloc\n", err);
> -               ib_umem_release(umem);
> -               goto err1;
> +               pr_warn("%s: Unable to allocate memory for map\n",
> +                               __func__);
> +               goto err_release_umem;
>         }
>
>         mr->page_shift = PAGE_SHIFT;
>         mr->page_mask = PAGE_SIZE - 1;
>
> -       num_buf                 = 0;
> +       num_buf = 0;
>         map = mr->map;
>         if (length > 0) {
>                 buf = map[0]->buf;
> @@ -151,10 +152,10 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
>
>                         vaddr = page_address(sg_page_iter_page(&sg_iter));
>                         if (!vaddr) {
> -                               pr_warn("null vaddr\n");
> -                               ib_umem_release(umem);
> +                               pr_warn("%s: Unable to get virtual address\n",
> +                                               __func__);
>                                 err = -ENOMEM;
> -                               goto err1;
> +                               goto err_cleanup_map;
>                         }
>
>                         buf->addr = (uintptr_t)vaddr;
> @@ -177,7 +178,13 @@ int rxe_mr_init_user(struct rxe_pd *pd, u64 start, u64 length, u64 iova,
>
>         return 0;
>
> -err1:
> +err_cleanup_map:
> +       for (i = 0; i < mr->num_map; i++)
> +               kfree(mr->map[i]);
> +       kfree(mr->map);
> +err_release_umem:
> +       ib_umem_release(umem);
> +err_out:
>         return err;
>  }
>
> --
> 2.30.2
>

  parent reply	other threads:[~2021-07-05  3:42 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-04 22:35 Bob Pearson
2021-07-05  2:09 ` yangx.jy
2021-07-05  3:42 ` Zhu Yanjun [this message]
2021-07-05  8:16   ` Haakon Bugge
2021-07-05  8:35     ` Zhu Yanjun
2021-07-05 15:40       ` Robert Pearson
2021-07-05 15:41         ` Robert Pearson
2021-07-05 15:54         ` Jason Gunthorpe
2021-07-05 16:16         ` Haakon Bugge

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='CAD=hENeHRjL8YhjwWi-dnknFAJeDUyHK3s-TdQf2AF853MHCMw@mail.gmail.com' \
    --to=zyjzyj2000@gmail.com \
    --cc=haakon.brugge@oracle.com \
    --cc=jgg@nvidia.com \
    --cc=linux-rdma@vger.kernel.org \
    --cc=rpearsonhpe@gmail.com \
    --subject='Re: [PATCH for-next] RDMA/rxe: Fix memory leak in error path code' \
    /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

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.