| Message ID | 20220526084411.480472-1-niejianglei2021@163.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | net/mlx5: Fix memory leak in mlx5_sf_dev_add() | expand |
On Thu, 2022-05-26 at 16:44 +0800, Jianglei Nie wrote: > The variable id is allocated by mlx5_adev_idx_alloc(). When some error > happens, the id should be freed by mlx5_adev_idx_free().But when > auxiliary_device_add() and xa_insert() fail, the id is not freed,which > will lead to a potential memory leak. > > We can fix it by calling mlx5_sf_dev_add() when auxiliary_device_add() > and xa_insert() fail. > > Signed-off-by: Jianglei Nie <niejianglei2021@163.com> > --- > drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > index 7da012ff0d41..9f222061a1c0 100644 > --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > @@ -125,13 +125,16 @@ static void mlx5_sf_dev_add(struct mlx5_core_dev *dev, u16 sf_index, u16 fn_id, > > err = auxiliary_device_add(&sf_dev->adev); > if (err) { > + mlx5_adev_idx_free(id); > put_device(&sf_dev->adev.dev); This looks not correct. put_device() -> mlx5_sf_dev_release() -> should already take care of that. > goto add_err; > } > > err = xa_insert(&table->devices, sf_index, sf_dev, GFP_KERNEL); > - if (err) > + if (err) { > + mlx5_adev_idx_free(id); > goto xa_err; > + } > return; > > xa_err:
> From: Paolo Abeni <pabeni@redhat.com> > Sent: Thursday, May 26, 2022 7:14 AM > > On Thu, 2022-05-26 at 16:44 +0800, Jianglei Nie wrote: > > The variable id is allocated by mlx5_adev_idx_alloc(). When some error > > happens, the id should be freed by mlx5_adev_idx_free().But when > > auxiliary_device_add() and xa_insert() fail, the id is not freed,which > > will lead to a potential memory leak. > > > > We can fix it by calling mlx5_sf_dev_add() when auxiliary_device_add() > > and xa_insert() fail. > > > > Signed-off-by: Jianglei Nie <niejianglei2021@163.com> > > --- > > drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > > b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > > index 7da012ff0d41..9f222061a1c0 100644 > > --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > > +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c > > @@ -125,13 +125,16 @@ static void mlx5_sf_dev_add(struct > mlx5_core_dev > > *dev, u16 sf_index, u16 fn_id, > > > > err = auxiliary_device_add(&sf_dev->adev); > > if (err) { > > + mlx5_adev_idx_free(id); > > put_device(&sf_dev->adev.dev); > > This looks not correct. put_device() -> mlx5_sf_dev_release() -> should > already take care of that. > Right. As Paolo explained, current code doesn’t have the cited bug. > > goto add_err; > > } > > > > err = xa_insert(&table->devices, sf_index, sf_dev, GFP_KERNEL); > > - if (err) > > + if (err) { > > + mlx5_adev_idx_free(id); > > goto xa_err; > > + } > > return; > > > > xa_err:
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c index 7da012ff0d41..9f222061a1c0 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c @@ -125,13 +125,16 @@ static void mlx5_sf_dev_add(struct mlx5_core_dev *dev, u16 sf_index, u16 fn_id, err = auxiliary_device_add(&sf_dev->adev); if (err) { + mlx5_adev_idx_free(id); put_device(&sf_dev->adev.dev); goto add_err; } err = xa_insert(&table->devices, sf_index, sf_dev, GFP_KERNEL); - if (err) + if (err) { + mlx5_adev_idx_free(id); goto xa_err; + } return; xa_err:
The variable id is allocated by mlx5_adev_idx_alloc(). When some error happens, the id should be freed by mlx5_adev_idx_free().But when auxiliary_device_add() and xa_insert() fail, the id is not freed,which will lead to a potential memory leak. We can fix it by calling mlx5_sf_dev_add() when auxiliary_device_add() and xa_insert() fail. Signed-off-by: Jianglei Nie <niejianglei2021@163.com> --- drivers/net/ethernet/mellanox/mlx5/core/sf/dev/dev.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)