| Message ID | 20241014144250.38802-1-wanghai38@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c186b7a7f2387d9e09ad408420570be025b187c5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ethernet: rtsn: fix potential memory leak in rtsn_start_xmit() | expand |
Hello Wang, Thanks for finding this. On 2024-10-14 22:42:50 +0800, Wang Hai wrote: > The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb > in case of skb->len being too long, add dev_kfree_skb_any() to fix it. > > Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") > Signed-off-by: Wang Hai <wanghai38@huawei.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/net/ethernet/renesas/rtsn.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c > index f9f63c61d792..6b3f7fca8d15 100644 > --- a/drivers/net/ethernet/renesas/rtsn.c > +++ b/drivers/net/ethernet/renesas/rtsn.c > @@ -1057,6 +1057,7 @@ static netdev_tx_t rtsn_start_xmit(struct sk_buff *skb, struct net_device *ndev) > if (skb->len >= TX_DS) { > priv->stats.tx_dropped++; > priv->stats.tx_errors++; > + dev_kfree_skb_any(skb); > goto out; > } > > -- > 2.17.1 >
Hi Wang, On Mon, Oct 14, 2024 at 4:43 PM Wang Hai <wanghai38@huawei.com> wrote: > The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb > in case of skb->len being too long, add dev_kfree_skb_any() to fix it. > > Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") > Signed-off-by: Wang Hai <wanghai38@huawei.com> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/rtsn.c > +++ b/drivers/net/ethernet/renesas/rtsn.c > @@ -1057,6 +1057,7 @@ static netdev_tx_t rtsn_start_xmit(struct sk_buff *skb, struct net_device *ndev) > if (skb->len >= TX_DS) { > priv->stats.tx_dropped++; > priv->stats.tx_errors++; > + dev_kfree_skb_any(skb); > goto out; > } Does the same apply to the skb_put_padto() failure path below? drivers/net/ethernet/renesas/rtsn.c- if (skb_put_padto(skb, ETH_ZLEN)) drivers/net/ethernet/renesas/rtsn.c- goto out; Gr{oetje,eeting}s, Geert
On Tue, Oct 15, 2024 at 8:58 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Wang, > > On Mon, Oct 14, 2024 at 4:43 PM Wang Hai <wanghai38@huawei.com> wrote: > > The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb > > in case of skb->len being too long, add dev_kfree_skb_any() to fix it. > > > > Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") > > Signed-off-by: Wang Hai <wanghai38@huawei.com> > > Thanks for your patch! > > > --- a/drivers/net/ethernet/renesas/rtsn.c > > +++ b/drivers/net/ethernet/renesas/rtsn.c > > @@ -1057,6 +1057,7 @@ static netdev_tx_t rtsn_start_xmit(struct sk_buff *skb, struct net_device *ndev) > > if (skb->len >= TX_DS) { > > priv->stats.tx_dropped++; > > priv->stats.tx_errors++; > > + dev_kfree_skb_any(skb); > > goto out; > > } > > Does the same apply to the skb_put_padto() failure path below? > > drivers/net/ethernet/renesas/rtsn.c- if (skb_put_padto(skb, ETH_ZLEN)) > drivers/net/ethernet/renesas/rtsn.c- goto out; In case of error, skb_put_padto() frees the skb. /** * skb_put_padto - increase size and pad an skbuff up to a minimal size * @skb: buffer to pad * @len: minimal length * * Pads up a buffer to ensure the trailing bytes exist and are * blanked. If the buffer already contains sufficient data it * is untouched. Otherwise it is extended. Returns zero on * success. The skb is freed on error. */ static inline int __must_check skb_put_padto(struct sk_buff *skb, unsigned int len) { return __skb_put_padto(skb, len, true); }
On Mon, Oct 14, 2024 at 4:43 PM Wang Hai <wanghai38@huawei.com> wrote: > > The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb > in case of skb->len being too long, add dev_kfree_skb_any() to fix it. > > Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > drivers/net/ethernet/renesas/rtsn.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c > index f9f63c61d792..6b3f7fca8d15 100644 > --- a/drivers/net/ethernet/renesas/rtsn.c > +++ b/drivers/net/ethernet/renesas/rtsn.c > @@ -1057,6 +1057,7 @@ static netdev_tx_t rtsn_start_xmit(struct sk_buff *skb, struct net_device *ndev) > if (skb->len >= TX_DS) { > priv->stats.tx_dropped++; > priv->stats.tx_errors++; > + dev_kfree_skb_any(skb); > goto out; > } > Note this is dead code for this driver. This condition should never be hit with checks in upper layers, because TX_DS is bigger than device max mtu. Reviewed-by: Eric Dumazet <edumazet@google.com>
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 14 Oct 2024 22:42:50 +0800 you wrote: > The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb > in case of skb->len being too long, add dev_kfree_skb_any() to fix it. > > Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > drivers/net/ethernet/renesas/rtsn.c | 1 + > 1 file changed, 1 insertion(+) Here is the summary with links: - [net] net: ethernet: rtsn: fix potential memory leak in rtsn_start_xmit() https://git.kernel.org/netdev/net/c/c186b7a7f238 You are awesome, thank you!
diff --git a/drivers/net/ethernet/renesas/rtsn.c b/drivers/net/ethernet/renesas/rtsn.c index f9f63c61d792..6b3f7fca8d15 100644 --- a/drivers/net/ethernet/renesas/rtsn.c +++ b/drivers/net/ethernet/renesas/rtsn.c @@ -1057,6 +1057,7 @@ static netdev_tx_t rtsn_start_xmit(struct sk_buff *skb, struct net_device *ndev) if (skb->len >= TX_DS) { priv->stats.tx_dropped++; priv->stats.tx_errors++; + dev_kfree_skb_any(skb); goto out; }
The rtsn_start_xmit() returns NETDEV_TX_OK without freeing skb in case of skb->len being too long, add dev_kfree_skb_any() to fix it. Fixes: b0d3969d2b4d ("net: ethernet: rtsn: Add support for Renesas Ethernet-TSN") Signed-off-by: Wang Hai <wanghai38@huawei.com> --- drivers/net/ethernet/renesas/rtsn.c | 1 + 1 file changed, 1 insertion(+)