| Message ID | 20241011113908.43966-1-wanghai38@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: ethernet: aeroflex: fix potential memory leak in greth_start_xmit_gbit() | expand |
On 11.10.24 13:39, Wang Hai wrote: > The greth_start_xmit_gbit() returns NETDEV_TX_OK without freeing skb > in case of skb->len being too long, add dev_kfree_skb() to fix it. > > Fixes: d4c41139df6e ("net: Add Aeroflex Gaisler 10/100/1G Ethernet MAC driver") > Signed-off-by: Wang Hai <wanghai38@huawei.com> > --- > drivers/net/ethernet/aeroflex/greth.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/net/ethernet/aeroflex/greth.c b/drivers/net/ethernet/aeroflex/greth.c > index 27af7746d645..8f6835a710b9 100644 > --- a/drivers/net/ethernet/aeroflex/greth.c > +++ b/drivers/net/ethernet/aeroflex/greth.c > @@ -484,6 +484,7 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev) > > if (unlikely(skb->len > MAX_FRAME_SIZE)) { > dev->stats.tx_errors++; > + dev_kfree_skb(skb); > goto out; dev_kfree_skb(skb) is already part of the error handling, one line above the "out" label. Why don't you just add another label which includes dev_kfree_skb(skb) and goto that label? Gerhard
On 2024/10/12 4:41, Gerhard Engleder wrote: > On 11.10.24 13:39, Wang Hai wrote: >> The greth_start_xmit_gbit() returns NETDEV_TX_OK without freeing skb >> in case of skb->len being too long, add dev_kfree_skb() to fix it. >> >> Fixes: d4c41139df6e ("net: Add Aeroflex Gaisler 10/100/1G Ethernet >> MAC driver") >> Signed-off-by: Wang Hai <wanghai38@huawei.com> >> --- >> drivers/net/ethernet/aeroflex/greth.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/net/ethernet/aeroflex/greth.c >> b/drivers/net/ethernet/aeroflex/greth.c >> index 27af7746d645..8f6835a710b9 100644 >> --- a/drivers/net/ethernet/aeroflex/greth.c >> +++ b/drivers/net/ethernet/aeroflex/greth.c >> @@ -484,6 +484,7 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct >> net_device *dev) >> if (unlikely(skb->len > MAX_FRAME_SIZE)) { >> dev->stats.tx_errors++; >> + dev_kfree_skb(skb); >> goto out; > > dev_kfree_skb(skb) is already part of the error handling, one line above > the "out" label. Why don't you just add another label which includes > dev_kfree_skb(skb) and goto that label? > > Gerhard Hi, Gerhard. Thanks for the suggestion, I just sent the v2 version. [PATCH v2 net] net: ethernet: aeroflex: fix potential memory leak in greth_start_xmit_gbit()
diff --git a/drivers/net/ethernet/aeroflex/greth.c b/drivers/net/ethernet/aeroflex/greth.c index 27af7746d645..8f6835a710b9 100644 --- a/drivers/net/ethernet/aeroflex/greth.c +++ b/drivers/net/ethernet/aeroflex/greth.c @@ -484,6 +484,7 @@ greth_start_xmit_gbit(struct sk_buff *skb, struct net_device *dev) if (unlikely(skb->len > MAX_FRAME_SIZE)) { dev->stats.tx_errors++; + dev_kfree_skb(skb); goto out; }
The greth_start_xmit_gbit() returns NETDEV_TX_OK without freeing skb in case of skb->len being too long, add dev_kfree_skb() to fix it. Fixes: d4c41139df6e ("net: Add Aeroflex Gaisler 10/100/1G Ethernet MAC driver") Signed-off-by: Wang Hai <wanghai38@huawei.com> --- drivers/net/ethernet/aeroflex/greth.c | 1 + 1 file changed, 1 insertion(+)