| Message ID | 20241014145901.48940-1-wanghai38@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | fed07d3eb8a8d9fcc0e455175a89bc6445d6faed |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: bcmasp: fix potential memory leak in bcmasp_xmit() | expand |
On 10/14/24 07:59, Wang Hai wrote: > The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb > in case of mapping fails, add dev_kfree_skb() to fix it. > > Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > index 82768b0e9026..9ea16ef4139d 100644 > --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > @@ -322,6 +322,7 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev) > } > /* Rewind so we do not have a hole */ > spb_index = intf->tx_spb_index; > + dev_kfree_skb(skb); Similar reasoning to the change proposed to bcmsysport.c, we already have a private counter tracking DMA mapping errors, therefore I would consider using dev_consume_skb_any() here.
On 2024/10/15 1:14, Florian Fainelli wrote: > On 10/14/24 07:59, Wang Hai wrote: >> The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb >> in case of mapping fails, add dev_kfree_skb() to fix it. >> >> Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet >> controller") >> Signed-off-by: Wang Hai <wanghai38@huawei.com> >> --- >> drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c >> b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c >> index 82768b0e9026..9ea16ef4139d 100644 >> --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c >> +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c >> @@ -322,6 +322,7 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff >> *skb, struct net_device *dev) >> } >> /* Rewind so we do not have a hole */ >> spb_index = intf->tx_spb_index; >> + dev_kfree_skb(skb); > > Similar reasoning to the change proposed to bcmsysport.c, we already > have a private counter tracking DMA mapping errors, therefore I would > consider using dev_consume_skb_any() here. Hi, Florian. Thanks for the suggestion, I've resent v2. [PATCH v2 net] net: bcmasp: fix potential memory leak in bcmasp_xmit()
On Mon, 14 Oct 2024 10:14:59 -0700 Florian Fainelli wrote: > On 10/14/24 07:59, Wang Hai wrote: > > The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb > > in case of mapping fails, add dev_kfree_skb() to fix it. > > > > Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller") > > Signed-off-by: Wang Hai <wanghai38@huawei.com> > > --- > > drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > > index 82768b0e9026..9ea16ef4139d 100644 > > --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > > +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c > > @@ -322,6 +322,7 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev) > > } > > /* Rewind so we do not have a hole */ > > spb_index = intf->tx_spb_index; > > + dev_kfree_skb(skb); > > Similar reasoning to the change proposed to bcmsysport.c, we already > have a private counter tracking DMA mapping errors, therefore I would > consider using dev_consume_skb_any() here. .. but this one hasn't been pushed and I presume same treatment will apply?
On 10/14/24 07:59, Wang Hai wrote: > The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb > in case of mapping fails, add dev_kfree_skb() to fix it. > > Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller") > Signed-off-by: Wang Hai <wanghai38@huawei.com> Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 14 Oct 2024 22:59:01 +0800 you wrote: > The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb > in case of mapping fails, add dev_kfree_skb() to fix it. > > Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 + > 1 file changed, 1 insertion(+) Here is the summary with links: - [net] net: bcmasp: fix potential memory leak in bcmasp_xmit() https://git.kernel.org/netdev/net/c/fed07d3eb8a8 You are awesome, thank you!
diff --git a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c index 82768b0e9026..9ea16ef4139d 100644 --- a/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c +++ b/drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c @@ -322,6 +322,7 @@ static netdev_tx_t bcmasp_xmit(struct sk_buff *skb, struct net_device *dev) } /* Rewind so we do not have a hole */ spb_index = intf->tx_spb_index; + dev_kfree_skb(skb); return NETDEV_TX_OK; }
The bcmasp_xmit() returns NETDEV_TX_OK without freeing skb in case of mapping fails, add dev_kfree_skb() to fix it. Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller") Signed-off-by: Wang Hai <wanghai38@huawei.com> --- drivers/net/ethernet/broadcom/asp2/bcmasp_intf.c | 1 + 1 file changed, 1 insertion(+)