| Message ID | 1566361206-5135-1-git-send-email-wenwen@cs.uga.edu |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] net: pch_gbe: Fix memory leaks | expand |
From: Wenwen Wang <wenwen@cs.uga.edu> Date: Tue, 20 Aug 2019 23:20:05 -0500 > In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and > 'rx_old' are not deallocated, leading to memory leaks. To fix this issue, > move the free statements to the outside of the if() statement. > > Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> Something still is not right here. > diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c > index 1a3008e..cb43919 100644 > --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c > +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c > @@ -340,12 +340,10 @@ static int pch_gbe_set_ringparam(struct net_device *netdev, > goto err_setup_tx; > pch_gbe_free_rx_resources(adapter, rx_old); > pch_gbe_free_tx_resources(adapter, tx_old); > - kfree(tx_old); > - kfree(rx_old); > - adapter->rx_ring = rxdr; > - adapter->tx_ring = txdr; > err = pch_gbe_up(adapter); > } > + kfree(tx_old); > + kfree(rx_old); If the if() condition ending here is not taken, you cannot just free these two pointers. You are then leaking the memory which would normally be liberated by pch_gbe_free_rx_resources() and pch_gbe_free_tx_resources(). What's more, in this same situation, the rx_old->dma value is probably still programmed into the hardware, and therefore the device still could potentially DMA read/write to that memory. I think the fix here is not simple, and you will need to do more extensive research in order to fix this properly. I'm not applying this, sorry.
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c index 1a3008e..cb43919 100644 --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c @@ -340,12 +340,10 @@ static int pch_gbe_set_ringparam(struct net_device *netdev, goto err_setup_tx; pch_gbe_free_rx_resources(adapter, rx_old); pch_gbe_free_tx_resources(adapter, tx_old); - kfree(tx_old); - kfree(rx_old); - adapter->rx_ring = rxdr; - adapter->tx_ring = txdr; err = pch_gbe_up(adapter); } + kfree(tx_old); + kfree(rx_old); return err; err_setup_tx:
In pch_gbe_set_ringparam(), if netif_running() returns false, 'tx_old' and 'rx_old' are not deallocated, leading to memory leaks. To fix this issue, move the free statements to the outside of the if() statement. Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu> --- drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_ethtool.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)