[PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Stephen Hemminger]

Code review of the axgbe PMD identified several correctness bugs,
primarily around resource management on error paths in device
initialization and an out-of-bounds descriptor ring access.

Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
  - mac_addrs leaked when hash_mac_addrs allocation fails
  - hash_mac_addrs leaked when phy_init() fails
  - rte_intr_callback_register() return value unchecked
  - DMA reset failure via hw_if.exit() logged but not propagated

Patch 2 fixes wrapper_rx_desc_init() which only releases the
current queue on mbuf allocation failure, leaking all mbufs
from previously initialized queues.

Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close()
for the four mutexes created during init.

Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
status functions where desc[idx + offset] can exceed the ring
size. The offset is now folded into the index before masking.


Stephen Hemminger (4):
  net/axgbe: fix resource leaks in device init error paths
  net/axgbe: fix Rx queue leak on descriptor init failure
  net/axgbe: destroy mutexes on device close
  net/axgbe: fix descriptor status out-of-bounds access

 drivers/net/axgbe/axgbe_dev.c    |  5 ++++-
 drivers/net/axgbe/axgbe_ethdev.c | 26 ++++++++++++++++++++++++--
 drivers/net/axgbe/axgbe_rxtx.c   |  8 ++++----
 3 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.51.0

[PATCH 1/4] net/axgbe: fix resource leaks in device init error paths

[Stephen Hemminger]

Several error paths in eth_axgbe_dev_init() fail to release
previously allocated resources:

 - When hash_mac_addrs allocation fails, mac_addrs is leaked.
 - When phy_init() fails, hash_mac_addrs is leaked.
 - The return value of rte_intr_callback_register() is not
   checked, so a failure leaves the driver without interrupt
   handling but continuing as if everything is functional.
 - When the DMA software reset via hw_if.exit() fails, the
   error is only logged and initialization continues with the
   hardware in an undefined state.

Fix all error paths to properly clean up previously allocated
resources, check the interrupt registration return value, and
propagate the DMA reset failure.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_ethdev.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index cfcd880961..709a52acc8 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2359,6 +2359,8 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 		PMD_INIT_LOG(ERR,
 			     "Failed to allocate %d bytes needed to "
 			     "store MAC addresses", len);
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
 		return -ENOMEM;
 	}
 
@@ -2405,8 +2407,14 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 					  pdata->vdata->rx_max_fifo_size);
 	/* Issue software reset to DMA */
 	ret = pdata->hw_if.exit(pdata);
-	if (ret)
+	if (ret) {
 		PMD_DRV_LOG_LINE(ERR, "hw_if->exit EBUSY error");
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
+		return ret;
+	}
 
 	/* Set default configuration data */
 	axgbe_default_config(pdata);
@@ -2426,14 +2434,23 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 
 	ret = pdata->phy_if.phy_init(pdata);
 	if (ret) {
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
 		rte_free(eth_dev->data->mac_addrs);
 		eth_dev->data->mac_addrs = NULL;
 		return ret;
 	}
 
-	rte_intr_callback_register(pci_dev->intr_handle,
+	ret = rte_intr_callback_register(pci_dev->intr_handle,
 				   axgbe_dev_interrupt_handler,
 				   (void *)eth_dev);
+	if (ret) {
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
+		return ret;
+	}
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
-- 
2.51.0

[PATCH 2/4] net/axgbe: fix Rx queue leak on descriptor init failure

[Stephen Hemminger]

When wrapper_rx_desc_init() fails to allocate an mbuf for queue i,
only queue i is released. Queues 0 through i-1 have already been
fully populated with mbufs assigned to sw_ring entries and
programmed into hardware descriptors, but are never cleaned up.
This leaks all mbufs from the previously initialized queues.

Fix by releasing all previously initialized queues when an
allocation failure occurs.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_dev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 634d4ee4a5..ae2ae42021 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -1025,7 +1025,10 @@ static int wrapper_rx_desc_init(struct axgbe_port *pdata)
 			if (mbuf == NULL) {
 				PMD_DRV_LOG_LINE(ERR, "RX mbuf alloc failed queue_id = %u, idx = %d",
 					    (unsigned int)rxq->queue_id, j);
-				axgbe_dev_rx_queue_release(pdata->eth_dev, i);
+
+				do {
+					axgbe_dev_rx_queue_release(pdata->eth_dev, i);
+				} while (i-- > 0);
 				return -ENOMEM;
 			}
 			rxq->sw_ring[j] = mbuf;
-- 
2.51.0

Re: [PATCH 2/4] net/axgbe: fix Rx queue leak on descriptor init failure

[Stephen Hemminger]

On Wed, 18 Feb 2026 08:41:00 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> When wrapper_rx_desc_init() fails to allocate an mbuf for queue i,
> only queue i is released. Queues 0 through i-1 have already been
> fully populated with mbufs assigned to sw_ring entries and
> programmed into hardware descriptors, but are never cleaned up.
> This leaks all mbufs from the previously initialized queues.
> 
> Fix by releasing all previously initialized queues when an
> allocation failure occurs.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---

AI found off by one in its own patch...

Patch 2 has an off-by-one in the cleanup loop. 
The do-while releases queue i first (the failing queue), then loops while i-- > 0. 
Since 0 > 0 is false, queue 0 is never released — its mbufs leak. 
Should use i-- != 0 or a forward for-loop through 0..i.

[PATCH 3/4] net/axgbe: destroy mutexes on device close

[Stephen Hemminger]

Four pthread mutexes (xpcs_mutex, i2c_mutex, an_mutex, phy_mutex)
are initialized in eth_axgbe_dev_init() but never destroyed in
axgbe_dev_close(). This leaks kernel resources on every device
close/reopen cycle.

Add pthread_mutex_destroy() calls to the close path.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 709a52acc8..c8414998b4 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2482,6 +2482,11 @@ axgbe_dev_close(struct rte_eth_dev *eth_dev)
 	/* Disable all interrupts in the hardware */
 	XP_IOWRITE(pdata, XP_INT_EN, 0x0);
 
+	pthread_mutex_destroy(&pdata->xpcs_mutex);
+	pthread_mutex_destroy(&pdata->i2c_mutex);
+	pthread_mutex_destroy(&pdata->an_mutex);
+	pthread_mutex_destroy(&pdata->phy_mutex);
+
 	return 0;
 }
 
-- 
2.51.0

[PATCH 4/4] net/axgbe: fix descriptor status out-of-bounds access

[Stephen Hemminger]

Both axgbe_dev_rx_descriptor_status() and
axgbe_dev_tx_descriptor_status() compute the descriptor address as
desc[idx + offset] where idx is the masked ring position. When
idx + offset >= nb_desc, this reads past the end of the
descriptor ring buffer.

Fix by incorporating the offset into the index before masking,
using AXGBE_GET_DESC_IDX() which wraps with (nb_desc - 1).

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_rxtx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 51a1aeb0b9..6f750d6ede 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -1205,8 +1205,8 @@ axgbe_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 	if (offset >= rxq->nb_desc - rxq->dirty)
 		return RTE_ETH_RX_DESC_UNAVAIL;
 
-	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur);
-	desc = &rxq->desc[idx + offset];
+	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur + offset);
+	desc = &rxq->desc[idx];
 
 	if (!AXGMAC_GET_BITS_LE(desc->write.desc3, RX_NORMAL_DESC3, OWN))
 		return RTE_ETH_RX_DESC_DONE;
@@ -1228,8 +1228,8 @@ axgbe_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 	if (offset >= txq->nb_desc - txq->dirty)
 		return RTE_ETH_TX_DESC_UNAVAIL;
 
-	idx = AXGBE_GET_DESC_IDX(txq, txq->dirty + txq->free_batch_cnt - 1);
-	desc = &txq->desc[idx + offset];
+	idx = AXGBE_GET_DESC_IDX(txq, txq->dirty + txq->free_batch_cnt - 1 + offset);
+	desc = &txq->desc[idx];
 
 	if (!AXGMAC_GET_BITS_LE(desc->desc3, TX_NORMAL_DESC3, OWN))
 		return RTE_ETH_TX_DESC_DONE;
-- 
2.51.0

Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Stephen Hemminger]

On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
> 
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
> 
> Patch 2 fixes wrapper_rx_desc_init() which only releases the
> current queue on mbuf allocation failure, leaking all mbufs
> from previously initialized queues.
> 
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close()
> for the four mutexes created during init.
> 
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring
> size. The offset is now folded into the index before masking.


Waiting for ACK from maintainer. I don't have the hardware.

Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Stephen Hemminger]

On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
> 
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
> 
> Patch 2 fixes wrapper_rx_desc_init() which only releases the
> current queue on mbuf allocation failure, leaking all mbufs
> from previously initialized queues.
> 
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close()
> for the four mutexes created during init.
> 
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring
> size. The offset is now folded into the index before masking.
> 
> 
> Stephen Hemminger (4):
>   net/axgbe: fix resource leaks in device init error paths
>   net/axgbe: fix Rx queue leak on descriptor init failure
>   net/axgbe: destroy mutexes on device close
>   net/axgbe: fix descriptor status out-of-bounds access
> 
>  drivers/net/axgbe/axgbe_dev.c    |  5 ++++-
>  drivers/net/axgbe/axgbe_ethdev.c | 26 ++++++++++++++++++++++++--
>  drivers/net/axgbe/axgbe_rxtx.c   |  8 ++++----
>  3 files changed, 32 insertions(+), 7 deletions(-)
> 

Could I get a review of this, and test on real hardware please.

RE: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

These patches are currently under review and testing. I will get back to you shortly.

Regards,
Selwin

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Wednesday, February 25, 2026 10:23 PM
To: Sebastian, Selwin <Selwin.Sebastian@amd.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
>
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
>
> Patch 2 fixes wrapper_rx_desc_init() which only releases the current
> queue on mbuf allocation failure, leaking all mbufs from previously
> initialized queues.
>
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close() for
> the four mutexes created during init.
>
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring size.
> The offset is now folded into the index before masking.
>
>
> Stephen Hemminger (4):
>   net/axgbe: fix resource leaks in device init error paths
>   net/axgbe: fix Rx queue leak on descriptor init failure
>   net/axgbe: destroy mutexes on device close
>   net/axgbe: fix descriptor status out-of-bounds access
>
>  drivers/net/axgbe/axgbe_dev.c    |  5 ++++-
>  drivers/net/axgbe/axgbe_ethdev.c | 26 ++++++++++++++++++++++++--
>  drivers/net/axgbe/axgbe_rxtx.c   |  8 ++++----
>  3 files changed, 32 insertions(+), 7 deletions(-)
>

Could I get a review of this, and test on real hardware please.

Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Stephen Hemminger]

On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
> 
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
> 
> Patch 2 fixes wrapper_rx_desc_init() which only releases the
> current queue on mbuf allocation failure, leaking all mbufs
> from previously initialized queues.
> 
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close()
> for the four mutexes created during init.
> 
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring
> size. The offset is now folded into the index before masking.
> 
> 
> Stephen Hemminger (4):
>   net/axgbe: fix resource leaks in device init error paths
>   net/axgbe: fix Rx queue leak on descriptor init failure
>   net/axgbe: destroy mutexes on device close
>   net/axgbe: fix descriptor status out-of-bounds access

Since these were code review generated fixes, still holding off for
maintainer of this driver to verify

RE: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Ande, Venkat Kumar]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Stephen,


We are checking the patches and doing the verification on our AMD setup. We will provide update on this. If possible, please share us the AI tool details, which you are using and steps used that will help us for future patch submission and code review.


Regards,
Venkat

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Saturday, March 7, 2026 12:26 AM
To: Sebastian, Selwin <Selwin.Sebastian@amd.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
>
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
>
> Patch 2 fixes wrapper_rx_desc_init() which only releases the current
> queue on mbuf allocation failure, leaking all mbufs from previously
> initialized queues.
>
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close() for
> the four mutexes created during init.
>
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring size.
> The offset is now folded into the index before masking.
>
>
> Stephen Hemminger (4):
>   net/axgbe: fix resource leaks in device init error paths
>   net/axgbe: fix Rx queue leak on descriptor init failure
>   net/axgbe: destroy mutexes on device close
>   net/axgbe: fix descriptor status out-of-bounds access

Since these were code review generated fixes, still holding off for maintainer of this driver to verify

RE: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

[Sebastian, Selwin]

[AMD Official Use Only - AMD Internal Distribution Only]

Acked-by: Selwin Sebastian<selwin.sebastian@amd.com>

Regards,
Selwin

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org>
Sent: Saturday, March 7, 2026 12:26 AM
To: Sebastian, Selwin <Selwin.Sebastian@amd.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH 0/4] net/axgbe: fix resource leaks and OOB access

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Wed, 18 Feb 2026 08:40:58 -0800
Stephen Hemminger <stephen@networkplumber.org> wrote:

> Code review of the axgbe PMD identified several correctness bugs,
> primarily around resource management on error paths in device
> initialization and an out-of-bounds descriptor ring access.
>
> Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
>   - mac_addrs leaked when hash_mac_addrs allocation fails
>   - hash_mac_addrs leaked when phy_init() fails
>   - rte_intr_callback_register() return value unchecked
>   - DMA reset failure via hw_if.exit() logged but not propagated
>
> Patch 2 fixes wrapper_rx_desc_init() which only releases the current
> queue on mbuf allocation failure, leaking all mbufs from previously
> initialized queues.
>
> Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close() for
> the four mutexes created during init.
>
> Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
> status functions where desc[idx + offset] can exceed the ring size.
> The offset is now folded into the index before masking.
>
>
> Stephen Hemminger (4):
>   net/axgbe: fix resource leaks in device init error paths
>   net/axgbe: fix Rx queue leak on descriptor init failure
>   net/axgbe: destroy mutexes on device close
>   net/axgbe: fix descriptor status out-of-bounds access

Since these were code review generated fixes, still holding off for maintainer of this driver to verify

[PATCH v2 0/4] net/axgbe: fix resource leaks and OOB access

[Stephen Hemminger]

Code review of the axgbe PMD identified several correctness bugs,
primarily around resource management on error paths in device
initialization and an out-of-bounds descriptor ring access.

Patch 1 fixes multiple resource leak paths in eth_axgbe_dev_init():
  - mac_addrs leaked when hash_mac_addrs allocation fails
  - hash_mac_addrs leaked when phy_init() fails
  - rte_intr_callback_register() return value unchecked
  - DMA reset failure via hw_if.exit() logged but not propagated

Patch 2 fixes wrapper_rx_desc_init() which only releases the
current queue on mbuf allocation failure, leaking all mbufs
from previously initialized queues.

Patch 3 adds pthread_mutex_destroy() calls in axgbe_dev_close()
for the four mutexes created during init.

Patch 4 fixes an out-of-bounds read in both Rx and Tx descriptor
status functions where desc[idx + offset] can exceed the ring
size. The offset is now folded into the index before masking.

v2:
  - Patch 2: use rte_pktmbuf_alloc_bulk() instead of per-descriptor
    allocation.
  - Add Fixes: tags to all patches.

Stephen Hemminger (4):
  net/axgbe: fix resource leaks in device init error paths
  net/axgbe: fix Rx queue leak on descriptor init failure
  net/axgbe: destroy mutexes on device close
  net/axgbe: fix descriptor status out-of-bounds access

 drivers/net/axgbe/axgbe_dev.c    | 27 ++++++++++++---------------
 drivers/net/axgbe/axgbe_ethdev.c | 26 ++++++++++++++++++++++++--
 drivers/net/axgbe/axgbe_rxtx.c   |  8 ++++----
 3 files changed, 40 insertions(+), 21 deletions(-)

-- 
2.53.0

[PATCH v2 1/4] net/axgbe: fix resource leaks in device init error paths

[Stephen Hemminger]

Several error paths in eth_axgbe_dev_init() fail to release
previously allocated resources:

 - When hash_mac_addrs allocation fails, mac_addrs is leaked.
 - When phy_init() fails, hash_mac_addrs is leaked.
 - The return value of rte_intr_callback_register() is not
   checked, so a failure leaves the driver without interrupt
   handling but continuing as if everything is functional.
 - When the DMA software reset via hw_if.exit() fails, the
   error is only logged and initialization continues with the
   hardware in an undefined state.

Fix all error paths to properly clean up previously allocated
resources, check the interrupt registration return value, and
propagate the DMA reset failure.

Fixes: e01d9b2e980b ("net/axgbe: support unicast hash table for MAC address")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_ethdev.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index cfcd880961..709a52acc8 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2359,6 +2359,8 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 		PMD_INIT_LOG(ERR,
 			     "Failed to allocate %d bytes needed to "
 			     "store MAC addresses", len);
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
 		return -ENOMEM;
 	}
 
@@ -2405,8 +2407,14 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 					  pdata->vdata->rx_max_fifo_size);
 	/* Issue software reset to DMA */
 	ret = pdata->hw_if.exit(pdata);
-	if (ret)
+	if (ret) {
 		PMD_DRV_LOG_LINE(ERR, "hw_if->exit EBUSY error");
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
+		return ret;
+	}
 
 	/* Set default configuration data */
 	axgbe_default_config(pdata);
@@ -2426,14 +2434,23 @@ eth_axgbe_dev_init(struct rte_eth_dev *eth_dev)
 
 	ret = pdata->phy_if.phy_init(pdata);
 	if (ret) {
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
 		rte_free(eth_dev->data->mac_addrs);
 		eth_dev->data->mac_addrs = NULL;
 		return ret;
 	}
 
-	rte_intr_callback_register(pci_dev->intr_handle,
+	ret = rte_intr_callback_register(pci_dev->intr_handle,
 				   axgbe_dev_interrupt_handler,
 				   (void *)eth_dev);
+	if (ret) {
+		rte_free(eth_dev->data->hash_mac_addrs);
+		eth_dev->data->hash_mac_addrs = NULL;
+		rte_free(eth_dev->data->mac_addrs);
+		eth_dev->data->mac_addrs = NULL;
+		return ret;
+	}
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
 		     pci_dev->id.device_id);
-- 
2.53.0

[PATCH v2 2/4] net/axgbe: fix Rx queue leak on descriptor init failure

[Stephen Hemminger]

When wrapper_rx_desc_init() fails to allocate an mbuf for queue i,
only queue i is released. Queues 0 through i-1 have already been
fully populated with mbufs assigned to sw_ring entries and
programmed into hardware descriptors, but are never cleaned up.
This leaks all mbufs from the previously initialized queues.

Fix by allocating all needed buffers ahead of time with
rte_pktmbuf_alloc_bulk() and on failure unwind the queues
that are already setup.

Fixes: 7c4158a5b592 ("net/axgbe: add DMA programming and start/stop")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_dev.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_dev.c b/drivers/net/axgbe/axgbe_dev.c
index 482d3d8062..9eabe0dfa8 100644
--- a/drivers/net/axgbe/axgbe_dev.c
+++ b/drivers/net/axgbe/axgbe_dev.c
@@ -1011,13 +1011,11 @@ static void wrapper_tx_desc_init(struct axgbe_port *pdata)
 
 static int wrapper_rx_desc_init(struct axgbe_port *pdata)
 {
-	struct axgbe_rx_queue *rxq;
-	struct rte_mbuf *mbuf;
 	volatile union axgbe_rx_desc *desc;
 	unsigned int i, j;
 
 	for (i = 0; i < pdata->eth_dev->data->nb_rx_queues; i++) {
-		rxq = pdata->eth_dev->data->rx_queues[i];
+		struct axgbe_rx_queue *rxq = pdata->eth_dev->data->rx_queues[i];
 
 		/* Initialize software ring entries */
 		rxq->mbuf_alloc = 0;
@@ -1025,19 +1023,18 @@ static int wrapper_rx_desc_init(struct axgbe_port *pdata)
 		rxq->dirty = 0;
 		desc = AXGBE_GET_DESC_PT(rxq, 0);
 
+		if (!rte_pktmbuf_alloc_bulk(rxq->mb_pool, rxq->sw_ring, rxq->nb_desc)) {
+			PMD_DRV_LOG_LINE(ERR, "RX mbuf alloc failed queue_id = %u, nb_desc = %u",
+					 i, rxq->nb_desc);
+			for (unsigned int k = 0; k < i; k++)
+				axgbe_dev_rx_queue_release(pdata->eth_dev, k);
+			return -ENOMEM;
+		}
+
 		for (j = 0; j < rxq->nb_desc; j++) {
-			mbuf = rte_mbuf_raw_alloc(rxq->mb_pool);
-			if (mbuf == NULL) {
-				PMD_DRV_LOG_LINE(ERR, "RX mbuf alloc failed queue_id = %u, idx = %d",
-					    (unsigned int)rxq->queue_id, j);
-				axgbe_dev_rx_queue_release(pdata->eth_dev, i);
-				return -ENOMEM;
-			}
-			rxq->sw_ring[j] = mbuf;
-			/* Mbuf populate */
-			mbuf->next = NULL;
-			mbuf->data_off = RTE_PKTMBUF_HEADROOM;
-			mbuf->nb_segs = 1;
+			struct rte_mbuf *mbuf = rxq->sw_ring[j];
+
+			/* mbuf is in reset state (nb_segs = 1, headroom, etc)  */
 			mbuf->port = rxq->port_id;
 			desc->read.baddr =
 				rte_cpu_to_le_64(
-- 
2.53.0

[PATCH v2 3/4] net/axgbe: destroy mutexes on device close

[Stephen Hemminger]

Four pthread mutexes (xpcs_mutex, i2c_mutex, an_mutex, phy_mutex)
are initialized in eth_axgbe_dev_init() but never destroyed in
axgbe_dev_close(). This leaks kernel resources on every device
close/reopen cycle.

Add pthread_mutex_destroy() calls to the close path.

Fixes: 572890ef6625 ("net/axgbe: add structs for MAC init and reset")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/axgbe/axgbe_ethdev.c b/drivers/net/axgbe/axgbe_ethdev.c
index 709a52acc8..c8414998b4 100644
--- a/drivers/net/axgbe/axgbe_ethdev.c
+++ b/drivers/net/axgbe/axgbe_ethdev.c
@@ -2482,6 +2482,11 @@ axgbe_dev_close(struct rte_eth_dev *eth_dev)
 	/* Disable all interrupts in the hardware */
 	XP_IOWRITE(pdata, XP_INT_EN, 0x0);
 
+	pthread_mutex_destroy(&pdata->xpcs_mutex);
+	pthread_mutex_destroy(&pdata->i2c_mutex);
+	pthread_mutex_destroy(&pdata->an_mutex);
+	pthread_mutex_destroy(&pdata->phy_mutex);
+
 	return 0;
 }
 
-- 
2.53.0

[PATCH v2 4/4] net/axgbe: fix descriptor status out-of-bounds access

[Stephen Hemminger]

Both axgbe_dev_rx_descriptor_status() and
axgbe_dev_tx_descriptor_status() compute the descriptor address as
desc[idx + offset] where idx is the masked ring position. When
idx + offset >= nb_desc, this reads past the end of the
descriptor ring buffer.

Fix by incorporating the offset into the index before masking,
using AXGBE_GET_DESC_IDX() which wraps with (nb_desc - 1).

Fixes: 0962b6055c08 ("net/axgbe: support descriptor status")
Cc: stable@dpdk.org

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 drivers/net/axgbe/axgbe_rxtx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index 51a1aeb0b9..6f750d6ede 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -1205,8 +1205,8 @@ axgbe_dev_rx_descriptor_status(void *rx_queue, uint16_t offset)
 	if (offset >= rxq->nb_desc - rxq->dirty)
 		return RTE_ETH_RX_DESC_UNAVAIL;
 
-	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur);
-	desc = &rxq->desc[idx + offset];
+	idx = AXGBE_GET_DESC_IDX(rxq, rxq->cur + offset);
+	desc = &rxq->desc[idx];
 
 	if (!AXGMAC_GET_BITS_LE(desc->write.desc3, RX_NORMAL_DESC3, OWN))
 		return RTE_ETH_RX_DESC_DONE;
@@ -1228,8 +1228,8 @@ axgbe_dev_tx_descriptor_status(void *tx_queue, uint16_t offset)
 	if (offset >= txq->nb_desc - txq->dirty)
 		return RTE_ETH_TX_DESC_UNAVAIL;
 
-	idx = AXGBE_GET_DESC_IDX(txq, txq->dirty + txq->free_batch_cnt - 1);
-	desc = &txq->desc[idx + offset];
+	idx = AXGBE_GET_DESC_IDX(txq, txq->dirty + txq->free_batch_cnt - 1 + offset);
+	desc = &txq->desc[idx];
 
 	if (!AXGMAC_GET_BITS_LE(desc->desc3, TX_NORMAL_DESC3, OWN))
 		return RTE_ETH_TX_DESC_DONE;
-- 
2.53.0