[PATCH net 0/2] ibmveth: fix open-fail unwind after LAN registration

[PATCH net 0/2] ibmveth: fix open-fail unwind after LAN registration

[Mingming Cao]

Hi,

ibmveth_open() has two independent unwind holes after the logical LAN
is set up. Neither needs the MQ RX series. This posting is against
current net.git.

Patch 1 issues h_free_logical_lan() on every post-register failure.
A pool allocation failure jumped to out_free_buffer_pools without
the hypercall, so PHYP still owned the buffer-list page when it was
unmapped. request_irq() failure already issued H_FREE before taking
the same label.

  Fixes: d43732ce021f ("ibmveth: properly unwind on init errors")

Patch 2 gives TX LTBs their own walk. out_free_buffer_pools reuses
open()'s loop index, so after the pool unwind i is -1 and the TX
buffers leak. request_irq() failure has the same leak. The TX-fail
goto on that walk also skips dma_unmap of the filter list
(pre-existing since the LTB was added).

  Fixes: d926793c1de9 ("ibmveth: Implement multi queue on xmit")

The MQ RX series on net-next keeps the helper versions of these
guards and does not depend on this pair. If both land, keep the
helpers in that series; these two patches are the current
single-queue open-fail path only.

Mingming

Mingming Cao (2):
  ibmveth: h_free logical LAN on open-fail after register
  ibmveth: fix TX LTB and filter unwind on open-fail

 drivers/net/ethernet/ibm/ibmveth.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

base-commit: 11536ee3d3e0b1bd35b6f3f8df55a6053eb0c71d
-- 
2.50.1 (Apple Git-155)

[PATCH net 1/2] ibmveth: h_free logical LAN on open-fail after register

[Mingming Cao]

ibmveth_open() registers the logical LAN, then allocates the RX
buffer pools. A pool failure jumps to out_free_buffer_pools without
h_free_logical_lan(), so PHYP still owns the buffer-list page when
it is unmapped. request_irq() failure already issued the hypercall
before taking the same label.

Move that h_free to the shared post-register unwind so both paths
deregister before the buffer list is unmapped.

Fixes: d43732ce021f ("ibmveth: properly unwind on init errors")
Cc: stable@vger.kernel.org
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmveth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 73e051d26b9d..3e44f6b714d4 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -718,10 +718,6 @@ static int ibmveth_open(struct net_device *netdev)
 	if (rc != 0) {
 		netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
 			   netdev->irq, rc);
-		do {
-			lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
-		} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
-
 		goto out_free_buffer_pools;
 	}
 
@@ -737,6 +733,9 @@ static int ibmveth_open(struct net_device *netdev)
 	return 0;
 
 out_free_buffer_pools:
+	do {
+		lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
+	} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
 	while (--i >= 0) {
 		if (adapter->rx_buff_pool[i].active)
 			ibmveth_free_buffer_pool(adapter,
-- 
2.50.1 (Apple Git-155)

[PATCH net 2/2] ibmveth: fix TX LTB and filter unwind on open-fail

[Mingming Cao]

out_free_buffer_pools reuses open()'s loop index for the TX LTB
walk. After the pool unwind i is -1, so the TX buffers allocated
earlier are leaked. request_irq() failure has the same leak.

The TX-fail goto on that walk also skips dma_unmap of the filter
list (pre-existing since the LTB was added). Rewrite the TX walk
to real_num_tx_queues with a pointer check, and unmap the filter
list before those frees.

Fixes: d926793c1de9 ("ibmveth: Implement multi queue on xmit")
Cc: stable@vger.kernel.org
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmveth.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 3e44f6b714d4..abebdb1fc262 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -553,10 +553,15 @@ static int ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter,
 
 static void ibmveth_free_tx_ltb(struct ibmveth_adapter *adapter, int idx)
 {
+	void *ptr = adapter->tx_ltb_ptr[idx];
+
+	if (!ptr)
+		return;
+
+	adapter->tx_ltb_ptr[idx] = NULL;
 	dma_unmap_single(&adapter->vdev->dev, adapter->tx_ltb_dma[idx],
 			 adapter->tx_ltb_size, DMA_TO_DEVICE);
-	kfree(adapter->tx_ltb_ptr[idx]);
-	adapter->tx_ltb_ptr[idx] = NULL;
+	kfree(ptr);
 }
 
 static int ibmveth_allocate_tx_ltb(struct ibmveth_adapter *adapter, int idx)
@@ -667,7 +672,7 @@ static int ibmveth_open(struct net_device *netdev)
 
 	for (i = 0; i < netdev->real_num_tx_queues; i++) {
 		if (ibmveth_allocate_tx_ltb(adapter, i))
-			goto out_free_tx_ltb;
+			goto out_unmap_filter_list;
 	}
 
 	adapter->rx_queue.index = 0;
@@ -745,10 +750,8 @@ static int ibmveth_open(struct net_device *netdev)
 	dma_unmap_single(dev, adapter->filter_list_dma, 4096,
 			 DMA_BIDIRECTIONAL);
 
-out_free_tx_ltb:
-	while (--i >= 0) {
+	for (i = netdev->real_num_tx_queues - 1; i >= 0; i--)
 		ibmveth_free_tx_ltb(adapter, i);
-	}
 
 out_unmap_buffer_list:
 	dma_unmap_single(dev, adapter->buffer_list_dma, 4096,
-- 
2.50.1 (Apple Git-155)

Re: [PATCH net 1/2] ibmveth: h_free logical LAN on open-fail after register

[Dave Marquardt]

Mingming Cao <mmc@linux.ibm.com> writes:

> ibmveth_open() registers the logical LAN, then allocates the RX
> buffer pools. A pool failure jumps to out_free_buffer_pools without
> h_free_logical_lan(), so PHYP still owns the buffer-list page when
> it is unmapped. request_irq() failure already issued the hypercall
> before taking the same label.
>
> Move that h_free to the shared post-register unwind so both paths
> deregister before the buffer list is unmapped.
>
> Fixes: d43732ce021f ("ibmveth: properly unwind on init errors")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 73e051d26b9d..3e44f6b714d4 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -718,10 +718,6 @@ static int ibmveth_open(struct net_device *netdev)
>  	if (rc != 0) {
>  		netdev_err(netdev, "unable to request irq 0x%x, rc %d\n",
>  			   netdev->irq, rc);
> -		do {
> -			lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
> -		} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
> -
>  		goto out_free_buffer_pools;
>  	}

This code change is good. But I noticed another thing that's a pretty
minor bug of useless code just after this:

	rc = -ENOMEM;

rc isn't used in that code path after it is set. Is there some reason I
do not understand to have this here?

> @@ -737,6 +733,9 @@ static int ibmveth_open(struct net_device *netdev)
>  	return 0;
>  
>  out_free_buffer_pools:
> +	do {
> +		lpar_rc = h_free_logical_lan(adapter->vdev->unit_address);
> +	} while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY));
>  	while (--i >= 0) {
>  		if (adapter->rx_buff_pool[i].active)
>  			ibmveth_free_buffer_pool(adapter,

Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>

Re: [PATCH net 2/2] ibmveth: fix TX LTB and filter unwind on open-fail

[Dave Marquardt]

Mingming Cao <mmc@linux.ibm.com> writes:

> out_free_buffer_pools reuses open()'s loop index for the TX LTB
> walk. After the pool unwind i is -1, so the TX buffers allocated
> earlier are leaked. request_irq() failure has the same leak.
>
> The TX-fail goto on that walk also skips dma_unmap of the filter
> list (pre-existing since the LTB was added). Rewrite the TX walk
> to real_num_tx_queues with a pointer check, and unmap the filter
> list before those frees.
>
> Fixes: d926793c1de9 ("ibmveth: Implement multi queue on xmit")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
> index 3e44f6b714d4..abebdb1fc262 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -553,10 +553,15 @@ static int ibmveth_rxq_harvest_buffer(struct ibmveth_adapter *adapter,
>  
>  static void ibmveth_free_tx_ltb(struct ibmveth_adapter *adapter, int idx)
>  {
> +	void *ptr = adapter->tx_ltb_ptr[idx];
> +
> +	if (!ptr)
> +		return;
> +
> +	adapter->tx_ltb_ptr[idx] = NULL;
>  	dma_unmap_single(&adapter->vdev->dev, adapter->tx_ltb_dma[idx],
>  			 adapter->tx_ltb_size, DMA_TO_DEVICE);
> -	kfree(adapter->tx_ltb_ptr[idx]);
> -	adapter->tx_ltb_ptr[idx] = NULL;
> +	kfree(ptr);
>  }
>  
>  static int ibmveth_allocate_tx_ltb(struct ibmveth_adapter *adapter, int idx)
> @@ -667,7 +672,7 @@ static int ibmveth_open(struct net_device *netdev)
>  
>  	for (i = 0; i < netdev->real_num_tx_queues; i++) {
>  		if (ibmveth_allocate_tx_ltb(adapter, i))
> -			goto out_free_tx_ltb;
> +			goto out_unmap_filter_list;
>  	}
>  
>  	adapter->rx_queue.index = 0;
> @@ -745,10 +750,8 @@ static int ibmveth_open(struct net_device *netdev)
>  	dma_unmap_single(dev, adapter->filter_list_dma, 4096,
>  			 DMA_BIDIRECTIONAL);
>  
> -out_free_tx_ltb:
> -	while (--i >= 0) {
> +	for (i = netdev->real_num_tx_queues - 1; i >= 0; i--)
>  		ibmveth_free_tx_ltb(adapter, i);
> -	}
>  
>  out_unmap_buffer_list:
>  	dma_unmap_single(dev, adapter->buffer_list_dma, 4096,

Reviewed-by: Dave Marquardt <davemarq@linux.ibm.com>

Re: [PATCH net 1/2] ibmveth: h_free logical LAN on open-fail after register

[mingming cao]

On 9/25/26 12:01 PM, Dave Marquardt wrote:
> This code change is good. But I noticed another thing that's a pretty
> minor bug of useless code just after this:
>
> 	rc = -ENOMEM;
>
> rc isn't used in that code path after it is set. Is there some reason I
> do not understand to have this here?

Yeah, that |rc = -ENOMEM| at line 728 is dead — set right before |return 
0| and never read again. Pre-existing junk. |d43732| set it for the 
bounce alloc that used to run next. |d6832| moved the TX LTB earlier and 
dropped the bounce, but left the line. After |request_irq| succeeds we 
replenish, start TX, and return 0.

It's harmless (not user-visible) — I prefer leaving it out of this Fixes 
and letting the MQ series clean it up naturally when it rewrites that 
whole open() path. But fine to respin a new version if desired.

Thanks for the review.

Mingming

Re: [PATCH net 1/2] ibmveth: h_free logical LAN on open-fail after register

[Dave Marquardt]

mingming cao <mmc@linux.ibm.com> writes:

> On 9/25/26 12:01 PM, Dave Marquardt wrote:
>> This code change is good. But I noticed another thing that's a pretty
>> minor bug of useless code just after this:
>>
>> 	rc = -ENOMEM;
>>
>> rc isn't used in that code path after it is set. Is there some reason I
>> do not understand to have this here?
>
> Yeah, that |rc = -ENOMEM| at line 728 is dead — set right before
> |return 0| and never read again. Pre-existing junk. |d43732| set it
> for the bounce alloc that used to run next. |d6832| moved the TX LTB
> earlier and dropped the bounce, but left the line. After |request_irq|
> succeeds we replenish, start TX, and return 0.
>
> It's harmless (not user-visible) — I prefer leaving it out of this
> Fixes and letting the MQ series clean it up naturally when it rewrites
> that whole open() path. But fine to respin a new version if desired.

Agreed.

-Dave

Re: [PATCH net 0/2] ibmveth: fix open-fail unwind after LAN registration

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 25 Sep 2026 11:05:28 -0700 you wrote:
> Hi,
> 
> ibmveth_open() has two independent unwind holes after the logical LAN
> is set up. Neither needs the MQ RX series. This posting is against
> current net.git.
> 
> Patch 1 issues h_free_logical_lan() on every post-register failure.
> A pool allocation failure jumped to out_free_buffer_pools without
> the hypercall, so PHYP still owned the buffer-list page when it was
> unmapped. request_irq() failure already issued H_FREE before taking
> the same label.
> 
> [...]

Here is the summary with links:
  - [net,1/2] ibmveth: h_free logical LAN on open-fail after register
    https://git.kernel.org/netdev/net/c/af0524bf4ce1
  - [net,2/2] ibmveth: fix TX LTB and filter unwind on open-fail
    https://git.kernel.org/netdev/net/c/84bec0bf0352

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html