drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 1 + 1 file changed, 1 insertion(+)
In the error handling of 'offset > adapter->ring_size', the
tx_ring->tx_buffer allocated by kzalloc should be freed,
instead of 'goto failed' instantly.
Fixes: a6a5325239c2 ("atl1e: Atheros L1E Gigabit Ethernet driver")
Signed-off-by: Zhipeng Lu <alexious@zju.edu.cn>
---
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
index 5935be190b9e..deb5a3f207cc 100644
--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c
@@ -866,6 +866,7 @@ static int atl1e_setup_ring_resources(struct atl1e_adapter *adapter)
netdev_err(adapter->netdev, "offset(%d) > ring size(%d) !!\n",
offset, adapter->ring_size);
err = -1;
+ kfree(tx_ring->tx_buffer);
goto failed;
}
--
2.34.1
>diff --git a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c >b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c >index 5935be190b9e..deb5a3f207cc 100644 >--- a/drivers/net/ethernet/atheros/atl1e/atl1e_main.c >+++ b/drivers/net/ethernet/atheros/atl1e/atl1e_main.c >@@ -866,6 +866,7 @@ static int atl1e_setup_ring_resources(struct >atl1e_adapter *adapter) > netdev_err(adapter->netdev, "offset(%d) > ring size(%d) !!\n", > offset, adapter->ring_size); > err = -1; >+ kfree(tx_ring->tx_buffer); [Suman] I think we should do tx_ring->tx_buffer = NULL also, to avoid use after free? > goto failed; > } > >-- >2.34.1 >
On Thu, 7 Dec 2023 17:08:15 +0000 Suman Ghosh wrote: > >+ kfree(tx_ring->tx_buffer); > > [Suman] I think we should do tx_ring->tx_buffer = NULL also, to avoid use after free? It's up to the driver. Some may call that defensive programming.
>On Thu, 7 Dec 2023 17:08:15 +0000 Suman Ghosh wrote: >> >+ kfree(tx_ring->tx_buffer); >> >> [Suman] I think we should do tx_ring->tx_buffer = NULL also, to avoid >use after free? > >It's up to the driver. Some may call that defensive programming. [Suman] Agree. I pointed it out since this driver is using this approach at other places. But sure, it is up to Zhipeng.
> >On Thu, 7 Dec 2023 17:08:15 +0000 Suman Ghosh wrote: > >> >+ kfree(tx_ring->tx_buffer); > >> > >> [Suman] I think we should do tx_ring->tx_buffer = NULL also, to avoid > >use after free? > > > >It's up to the driver. Some may call that defensive programming. > [Suman] Agree. I pointed it out since this driver is using this approach at other places. But sure, it is up to Zhipeng. [Zhipeng] I think Suman's suggestion is valuable, it prevents potiential use-after-free and is consistent with other free operations in the same module.
© 2016 - 2025 Red Hat, Inc.