* [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow
@ 2018-06-07 19:19 Gustavo A. R. Silva
2018-06-08 23:08 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Gustavo A. R. Silva @ 2018-06-07 19:19 UTC (permalink / raw)
To: Raed Salem, Leon Romanovsky, Doug Ledford, Jason Gunthorpe
Cc: linux-rdma, linux-kernel, Gustavo A. R. Silva
In case memory resources for *ucmd* were allocated, release them
before return.
Addresses-Coverity-ID: 1469857 ("Resource leak")
Fixes: 3b3233fbf02e ("IB/mlx5: Add flow counters binding support")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
drivers/infiniband/hw/mlx5/main.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index e52dd21..0472e6c 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -3546,29 +3546,35 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
return ERR_PTR(-ENOMEM);
err = ib_copy_from_udata(ucmd, udata, required_ucmd_sz);
- if (err) {
- kfree(ucmd);
- return ERR_PTR(err);
- }
+ if (err)
+ goto free_ucmd;
}
- if (flow_attr->priority > MLX5_IB_FLOW_LAST_PRIO)
- return ERR_PTR(-ENOMEM);
+ if (flow_attr->priority > MLX5_IB_FLOW_LAST_PRIO) {
+ err = -ENOMEM;
+ goto free_ucmd;
+ }
if (domain != IB_FLOW_DOMAIN_USER ||
flow_attr->port > dev->num_ports ||
(flow_attr->flags & ~(IB_FLOW_ATTR_FLAGS_DONT_TRAP |
- IB_FLOW_ATTR_FLAGS_EGRESS)))
- return ERR_PTR(-EINVAL);
+ IB_FLOW_ATTR_FLAGS_EGRESS))) {
+ err = -EINVAL;
+ goto free_ucmd;
+ }
if (is_egress &&
(flow_attr->type == IB_FLOW_ATTR_ALL_DEFAULT ||
- flow_attr->type == IB_FLOW_ATTR_MC_DEFAULT))
- return ERR_PTR(-EINVAL);
+ flow_attr->type == IB_FLOW_ATTR_MC_DEFAULT)) {
+ err = -EINVAL;
+ goto free_ucmd;
+ }
dst = kzalloc(sizeof(*dst), GFP_KERNEL);
- if (!dst)
- return ERR_PTR(-ENOMEM);
+ if (!dst) {
+ err = -ENOMEM;
+ goto free_ucmd;
+ }
mutex_lock(&dev->flow_db->lock);
@@ -3640,6 +3646,9 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
kfree(ucmd);
kfree(handler);
return ERR_PTR(err);
+free_ucmd:
+ kfree(ucmd);
+ return ERR_PTR(err);
}
static u32 mlx5_ib_flow_action_flags_to_accel_xfrm_flags(u32 mlx5_flags)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow
2018-06-07 19:19 [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow Gustavo A. R. Silva
@ 2018-06-08 23:08 ` Jason Gunthorpe
2018-06-08 23:11 ` Gustavo A. R. Silva
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2018-06-08 23:08 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Raed Salem, Leon Romanovsky, Doug Ledford, linux-rdma,
linux-kernel
On Thu, Jun 07, 2018 at 02:19:15PM -0500, Gustavo A. R. Silva wrote:
> In case memory resources for *ucmd* were allocated, release them
> before return.
>
> Addresses-Coverity-ID: 1469857 ("Resource leak")
> Fixes: 3b3233fbf02e ("IB/mlx5: Add flow counters binding support")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> drivers/infiniband/hw/mlx5/main.c | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
> index e52dd21..0472e6c 100644
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -3546,29 +3546,35 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
> return ERR_PTR(-ENOMEM);
>
> err = ib_copy_from_udata(ucmd, udata, required_ucmd_sz);
> - if (err) {
> - kfree(ucmd);
> - return ERR_PTR(err);
> - }
> + if (err)
> + goto free_ucmd;
> }
>
> - if (flow_attr->priority > MLX5_IB_FLOW_LAST_PRIO)
> - return ERR_PTR(-ENOMEM);
> + if (flow_attr->priority > MLX5_IB_FLOW_LAST_PRIO) {
> + err = -ENOMEM;
> + goto free_ucmd;
> + }
>
> if (domain != IB_FLOW_DOMAIN_USER ||
> flow_attr->port > dev->num_ports ||
> (flow_attr->flags & ~(IB_FLOW_ATTR_FLAGS_DONT_TRAP |
> - IB_FLOW_ATTR_FLAGS_EGRESS)))
> - return ERR_PTR(-EINVAL);
> + IB_FLOW_ATTR_FLAGS_EGRESS))) {
> + err = -EINVAL;
> + goto free_ucmd;
> + }
>
> if (is_egress &&
> (flow_attr->type == IB_FLOW_ATTR_ALL_DEFAULT ||
> - flow_attr->type == IB_FLOW_ATTR_MC_DEFAULT))
> - return ERR_PTR(-EINVAL);
> + flow_attr->type == IB_FLOW_ATTR_MC_DEFAULT)) {
> + err = -EINVAL;
> + goto free_ucmd;
> + }
>
> dst = kzalloc(sizeof(*dst), GFP_KERNEL);
> - if (!dst)
> - return ERR_PTR(-ENOMEM);
> + if (!dst) {
> + err = -ENOMEM;
> + goto free_ucmd;
> + }
>
> mutex_lock(&dev->flow_db->lock);
>
> @@ -3640,6 +3646,9 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
> kfree(ucmd);
> kfree(handler);
> return ERR_PTR(err);
> +free_ucmd:
> + kfree(ucmd);
> + return ERR_PTR(err);
> }
This hunk is a bit wonky, can we do this instead? handle never needs
to be freed.
destroy_ft:
put_flow_table(dev, ft_prio, false);
if (ft_prio_tx)
put_flow_table(dev, ft_prio_tx, false);
unlock:
mutex_unlock(&dev->flow_db->lock);
kfree(dst);
free_ucmd:
kfree(ucmd);
return ERR_PTR(err);
}
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow
2018-06-08 23:08 ` Jason Gunthorpe
@ 2018-06-08 23:11 ` Gustavo A. R. Silva
2018-06-08 23:21 ` Jason Gunthorpe
0 siblings, 1 reply; 5+ messages in thread
From: Gustavo A. R. Silva @ 2018-06-08 23:11 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Raed Salem, Leon Romanovsky, Doug Ledford, linux-rdma,
linux-kernel
>>
>> mutex_lock(&dev->flow_db->lock);
>>
>> @@ -3640,6 +3646,9 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
>> kfree(ucmd);
>> kfree(handler);
>> return ERR_PTR(err);
>> +free_ucmd:
>> + kfree(ucmd);
>> + return ERR_PTR(err);
>> }
>
> This hunk is a bit wonky, can we do this instead? handle never needs
> to be freed.
>
> destroy_ft:
> put_flow_table(dev, ft_prio, false);
> if (ft_prio_tx)
> put_flow_table(dev, ft_prio_tx, false);
> unlock:
> mutex_unlock(&dev->flow_db->lock);
> kfree(dst);
> free_ucmd:
> kfree(ucmd);
> return ERR_PTR(err);
> }
>
>
Sure thing. I'll send v2 shortly.
Thanks for the feedback.
--
Gustavo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow
2018-06-08 23:11 ` Gustavo A. R. Silva
@ 2018-06-08 23:21 ` Jason Gunthorpe
2018-06-08 23:24 ` Gustavo A. R. Silva
0 siblings, 1 reply; 5+ messages in thread
From: Jason Gunthorpe @ 2018-06-08 23:21 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Raed Salem, Leon Romanovsky, Doug Ledford, linux-rdma,
linux-kernel
On Fri, Jun 08, 2018 at 06:11:49PM -0500, Gustavo A. R. Silva wrote:
>
> >> mutex_lock(&dev->flow_db->lock);
> >>@@ -3640,6 +3646,9 @@ static struct ib_flow *mlx5_ib_create_flow(struct ib_qp *qp,
> >> kfree(ucmd);
> >> kfree(handler);
> >> return ERR_PTR(err);
> >>+free_ucmd:
> >>+ kfree(ucmd);
> >>+ return ERR_PTR(err);
> >> }
> >
> >This hunk is a bit wonky, can we do this instead? handle never needs
> >to be freed.
> >
> >destroy_ft:
> > put_flow_table(dev, ft_prio, false);
> > if (ft_prio_tx)
> > put_flow_table(dev, ft_prio_tx, false);
> >unlock:
> > mutex_unlock(&dev->flow_db->lock);
> > kfree(dst);
> >free_ucmd:
> > kfree(ucmd);
> > return ERR_PTR(err);
> >}
> >
> >
>
> Sure thing. I'll send v2 shortly.
I made the adjustment for you, if it is wrong let me know..
https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit/?h=wip/jgg-for-rc&id=2cc82dd58712888758c2d2b405d012f2cd580dd7
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-08 23:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-07 19:19 [PATCH] IB/mlx5: Fix memory leak in mlx5_ib_create_flow Gustavo A. R. Silva
2018-06-08 23:08 ` Jason Gunthorpe
2018-06-08 23:11 ` Gustavo A. R. Silva
2018-06-08 23:21 ` Jason Gunthorpe
2018-06-08 23:24 ` Gustavo A. R. Silva
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.