| Message ID | 20250305100950.4001113-1-haoxiang_li2024@163.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] qlcnic: fix a memory leak in qlcnic_sriov_set_guest_vlan_mode() | expand |
On 3/5/25 11:09 AM, Haoxiang Li wrote: > Add qlcnic_sriov_free_vlans() to free the memory allocated by > qlcnic_sriov_alloc_vlans() if qlcnic_sriov_alloc_vlans() fails > or "sriov->allowed_vlans" fails to be allocated. > > Fixes: 91b7282b613d ("qlcnic: Support VLAN id config.") > Cc: stable@vger.kernel.org > Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com> > --- > Changes in v2: > - Add qlcnic_sriov_free_vlans() if qlcnic_sriov_alloc_vlans() fails. > - Modify the patch description. > vf_info was allocated by kcalloc, no need to do more checks cause > kfree(NULL) is safe. Thanks, Paolo! > --- > drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c > index f9dd50152b1e..0dd9d7cb1de9 100644 > --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c > +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c > @@ -446,16 +446,20 @@ static int qlcnic_sriov_set_guest_vlan_mode(struct qlcnic_adapter *adapter, > sriov->num_allowed_vlans); > > ret = qlcnic_sriov_alloc_vlans(adapter); > - if (ret) > + if (ret) { > + qlcnic_sriov_free_vlans(adapter); I'm sorry for the lack of clarity in my previous reply. I think it would be better to do this cleanup inside qlcnic_sriov_alloc_vlans(), so that on error it returns with no vlan allocated. There is another caller of qlcnic_sriov_alloc_vlans() which AFAICS still leak memory on error. Handling the deallocation in qlcnic_sriov_alloc_vlans() will address even that caller. Thanks, Paolo
On 3/6/25 11:32 AM, Paolo Abeni wrote: > On 3/5/25 11:09 AM, Haoxiang Li wrote: >> Add qlcnic_sriov_free_vlans() to free the memory allocated by >> qlcnic_sriov_alloc_vlans() if qlcnic_sriov_alloc_vlans() fails >> or "sriov->allowed_vlans" fails to be allocated. >> >> Fixes: 91b7282b613d ("qlcnic: Support VLAN id config.") >> Cc: stable@vger.kernel.org >> Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com> Whoops, I forgot: please include the target tree name ('net' in this case) in the subj prefix - in the next iteration should be 'PATCH net v3'. Thanks, Paolo
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c index f9dd50152b1e..0dd9d7cb1de9 100644 --- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c +++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c @@ -446,16 +446,20 @@ static int qlcnic_sriov_set_guest_vlan_mode(struct qlcnic_adapter *adapter, sriov->num_allowed_vlans); ret = qlcnic_sriov_alloc_vlans(adapter); - if (ret) + if (ret) { + qlcnic_sriov_free_vlans(adapter); return ret; + } if (!sriov->any_vlan) return 0; num_vlans = sriov->num_allowed_vlans; sriov->allowed_vlans = kcalloc(num_vlans, sizeof(u16), GFP_KERNEL); - if (!sriov->allowed_vlans) + if (!sriov->allowed_vlans) { + qlcnic_sriov_free_vlans(adapter); return -ENOMEM; + } vlans = (u16 *)&cmd->rsp.arg[3]; for (i = 0; i < num_vlans; i++)
Add qlcnic_sriov_free_vlans() to free the memory allocated by qlcnic_sriov_alloc_vlans() if qlcnic_sriov_alloc_vlans() fails or "sriov->allowed_vlans" fails to be allocated. Fixes: 91b7282b613d ("qlcnic: Support VLAN id config.") Cc: stable@vger.kernel.org Signed-off-by: Haoxiang Li <haoxiang_li2024@163.com> --- Changes in v2: - Add qlcnic_sriov_free_vlans() if qlcnic_sriov_alloc_vlans() fails. - Modify the patch description. vf_info was allocated by kcalloc, no need to do more checks cause kfree(NULL) is safe. Thanks, Paolo! --- drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_common.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)