* [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove()
@ 2022-12-05 6:15 Yuan Can
2022-12-07 11:20 ` patchwork-bot+netdevbpf
2022-12-07 11:55 ` Vladimir Oltean
0 siblings, 2 replies; 5+ messages in thread
From: Yuan Can @ 2022-12-05 6:15 UTC (permalink / raw)
To: ioana.ciornei, davem, edumazet, kuba, pabeni, netdev; +Cc: yuancan
The cmd_buff needs to be freed when error happened in
dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove().
Fixes: 1110318d83e8 ("dpaa2-switch: add tc flower hardware offload on ingress traffic")
Signed-off-by: Yuan Can <yuancan@huawei.com>
---
drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c
index cacd454ac696..c39b866e2582 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c
@@ -132,6 +132,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block,
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, acl_entry_cfg->key_iova))) {
dev_err(dev, "DMA mapping failed\n");
+ kfree(cmd_buff);
return -EFAULT;
}
@@ -142,6 +143,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block,
DMA_TO_DEVICE);
if (err) {
dev_err(dev, "dpsw_acl_add_entry() failed %d\n", err);
+ kfree(cmd_buff);
return err;
}
@@ -172,6 +174,7 @@ dpaa2_switch_acl_entry_remove(struct dpaa2_switch_filter_block *block,
DMA_TO_DEVICE);
if (unlikely(dma_mapping_error(dev, acl_entry_cfg->key_iova))) {
dev_err(dev, "DMA mapping failed\n");
+ kfree(cmd_buff);
return -EFAULT;
}
@@ -182,6 +185,7 @@ dpaa2_switch_acl_entry_remove(struct dpaa2_switch_filter_block *block,
DMA_TO_DEVICE);
if (err) {
dev_err(dev, "dpsw_acl_remove_entry() failed %d\n", err);
+ kfree(cmd_buff);
return err;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() 2022-12-05 6:15 [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() Yuan Can @ 2022-12-07 11:20 ` patchwork-bot+netdevbpf 2022-12-07 11:55 ` Vladimir Oltean 1 sibling, 0 replies; 5+ messages in thread From: patchwork-bot+netdevbpf @ 2022-12-07 11:20 UTC (permalink / raw) To: Yuan Can; +Cc: ioana.ciornei, davem, edumazet, kuba, pabeni, netdev Hello: This patch was applied to netdev/net.git (master) by Paolo Abeni <pabeni@redhat.com>: On Mon, 5 Dec 2022 06:15:15 +0000 you wrote: > The cmd_buff needs to be freed when error happened in > dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove(). > > Fixes: 1110318d83e8 ("dpaa2-switch: add tc flower hardware offload on ingress traffic") > Signed-off-by: Yuan Can <yuancan@huawei.com> > --- > drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c | 4 ++++ > 1 file changed, 4 insertions(+) Here is the summary with links: - dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() https://git.kernel.org/netdev/net/c/4fad22a1281c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() 2022-12-05 6:15 [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() Yuan Can 2022-12-07 11:20 ` patchwork-bot+netdevbpf @ 2022-12-07 11:55 ` Vladimir Oltean 2022-12-08 1:51 ` Yuan Can 1 sibling, 1 reply; 5+ messages in thread From: Vladimir Oltean @ 2022-12-07 11:55 UTC (permalink / raw) To: Yuan Can; +Cc: ioana.ciornei, davem, edumazet, kuba, pabeni, netdev Hi Yuan, On Mon, Dec 05, 2022 at 06:15:15AM +0000, Yuan Can wrote: > The cmd_buff needs to be freed when error happened in > dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove(). > > Fixes: 1110318d83e8 ("dpaa2-switch: add tc flower hardware offload on ingress traffic") > Signed-off-by: Yuan Can <yuancan@huawei.com> > --- > drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c > index cacd454ac696..c39b866e2582 100644 > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c > @@ -132,6 +132,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block, > DMA_TO_DEVICE); > if (unlikely(dma_mapping_error(dev, acl_entry_cfg->key_iova))) { > dev_err(dev, "DMA mapping failed\n"); > + kfree(cmd_buff); > return -EFAULT; > } > > @@ -142,6 +143,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block, > DMA_TO_DEVICE); > if (err) { > dev_err(dev, "dpsw_acl_add_entry() failed %d\n", err); > + kfree(cmd_buff); To reduce the number of kfree() calls, this last one can be put right before checking for error, and we could remove the kfree(cmd_buff) call at the very end. I mean that was already the intention, if you look at the dma_unmap_single() call compared to the error checking. Like this: err = dpsw_acl_add_entry(...); dma_unmap_single(dev, acl_entry_cfg->key_iova, sizeof(cmd_buff), DMA_TO_DEVICE); kfree(cmd_buff); if (err) { dev_err(dev, "dpsw_acl_add_entry() failed %d\n", err); return err; } return 0; } > return err; > } > > @@ -172,6 +174,7 @@ dpaa2_switch_acl_entry_remove(struct dpaa2_switch_filter_block *block, > DMA_TO_DEVICE); > if (unlikely(dma_mapping_error(dev, acl_entry_cfg->key_iova))) { > dev_err(dev, "DMA mapping failed\n"); > + kfree(cmd_buff); > return -EFAULT; > } > > @@ -182,6 +185,7 @@ dpaa2_switch_acl_entry_remove(struct dpaa2_switch_filter_block *block, > DMA_TO_DEVICE); > if (err) { > dev_err(dev, "dpsw_acl_remove_entry() failed %d\n", err); > + kfree(cmd_buff); Similar here: err = dpsw_acl_remove_entry(ethsw->mc_io, 0, ethsw->dpsw_handle, block->acl_id, acl_entry_cfg); dma_unmap_single(dev, acl_entry_cfg->key_iova, sizeof(cmd_buff), DMA_TO_DEVICE); kfree(cmd_buff); if (err) { dev_err(dev, "dpsw_acl_remove_entry() failed %d\n", err); return err; } return 0; } > return err; > } > > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() 2022-12-07 11:55 ` Vladimir Oltean @ 2022-12-08 1:51 ` Yuan Can 2022-12-08 14:20 ` Vladimir Oltean 0 siblings, 1 reply; 5+ messages in thread From: Yuan Can @ 2022-12-08 1:51 UTC (permalink / raw) To: Vladimir Oltean; +Cc: ioana.ciornei, davem, edumazet, kuba, pabeni, netdev 在 2022/12/7 19:55, Vladimir Oltean 写道: > Hi Yuan, > > On Mon, Dec 05, 2022 at 06:15:15AM +0000, Yuan Can wrote: >> The cmd_buff needs to be freed when error happened in >> dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove(). >> >> Fixes: 1110318d83e8 ("dpaa2-switch: add tc flower hardware offload on ingress traffic") >> Signed-off-by: Yuan Can <yuancan@huawei.com> >> --- >> drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c >> index cacd454ac696..c39b866e2582 100644 >> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c >> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch-flower.c >> @@ -132,6 +132,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block, >> DMA_TO_DEVICE); >> if (unlikely(dma_mapping_error(dev, acl_entry_cfg->key_iova))) { >> dev_err(dev, "DMA mapping failed\n"); >> + kfree(cmd_buff); >> return -EFAULT; >> } >> >> @@ -142,6 +143,7 @@ int dpaa2_switch_acl_entry_add(struct dpaa2_switch_filter_block *filter_block, >> DMA_TO_DEVICE); >> if (err) { >> dev_err(dev, "dpsw_acl_add_entry() failed %d\n", err); >> + kfree(cmd_buff); > To reduce the number of kfree() calls, this last one can be put right > before checking for error, and we could remove the kfree(cmd_buff) call at > the very end. I mean that was already the intention, if you look at the > dma_unmap_single() call compared to the error checking. Like this: > > err = dpsw_acl_add_entry(...); > > dma_unmap_single(dev, acl_entry_cfg->key_iova, sizeof(cmd_buff), > DMA_TO_DEVICE); > kfree(cmd_buff); > > if (err) { > dev_err(dev, "dpsw_acl_add_entry() failed %d\n", err); > return err; > } > > return 0; > } Nice! Thanks for the suggestion, as the patch has been merged, let me send another patch to do this job. -- Best regards, Yuan Can ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() 2022-12-08 1:51 ` Yuan Can @ 2022-12-08 14:20 ` Vladimir Oltean 0 siblings, 0 replies; 5+ messages in thread From: Vladimir Oltean @ 2022-12-08 14:20 UTC (permalink / raw) To: Yuan Can; +Cc: ioana.ciornei, davem, edumazet, kuba, pabeni, netdev On Thu, Dec 08, 2022 at 09:51:45AM +0800, Yuan Can wrote: > Nice! Thanks for the suggestion, as the patch has been merged, let me send > another patch to do this job. The patch was merged at the same time as I was writing my reply. Since the patch went to the "net.git" tree where only bug fixes are accepted, any further rework would have to go to "net-next.git". A resynchronization (merge net into net-next) takes place weekly, one should take place later today or tomorrow, I think. So if you're going to send a follow-up patch, I'd wait until "net-next" contains your change. That being said, it's not all that critical to follow up with another patch. I simply hadn't noticed your patch was merged, but it does the job as well, just not in the same style as the rest of these 2 functions. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-08 14:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-05 6:15 [PATCH] dpaa2-switch: Fix memory leak in dpaa2_switch_acl_entry_add() and dpaa2_switch_acl_entry_remove() Yuan Can 2022-12-07 11:20 ` patchwork-bot+netdevbpf 2022-12-07 11:55 ` Vladimir Oltean 2022-12-08 1:51 ` Yuan Can 2022-12-08 14:20 ` Vladimir Oltean
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).