[PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[Jakub Kicinski]

The semantics are that caller of fbnic_mbx_map_msg() retains
the ownership of the message on error. All existing callers
dutifully free the page.

Fixes: da3cde08209e ("eth: fbnic: Add FW communication mechanism")
Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index edd4adad954a..72c688b17c5b 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -127,11 +127,8 @@ static int fbnic_mbx_map_msg(struct fbnic_dev *fbd, int mbx_idx,
 		return -EBUSY;
 
 	addr = dma_map_single(fbd->dev, msg, PAGE_SIZE, direction);
-	if (dma_mapping_error(fbd->dev, addr)) {
-		free_page((unsigned long)msg);
-
+	if (dma_mapping_error(fbd->dev, addr))
 		return -ENOSPC;
-	}
 
 	mbx->buf_info[tail].msg = msg;
 	mbx->buf_info[tail].addr = addr;
-- 
2.49.0

RE: [PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[Keller, Jacob E]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, June 16, 2025 12:55 PM
> To: davem@davemloft.net
> Cc: netdev@vger.kernel.org; edumazet@google.com; pabeni@redhat.com;
> andrew+netdev@lunn.ch; horms@kernel.org; lee@trager.us; Keller, Jacob E
> <jacob.e.keller@intel.com>; Jakub Kicinski <kuba@kernel.org>; Alexander Duyck
> <alexanderduyck@fb.com>
> Subject: [PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW
> msg
> 
> The semantics are that caller of fbnic_mbx_map_msg() retains
> the ownership of the message on error. All existing callers
> dutifully free the page.
> 
> Fixes: da3cde08209e ("eth: fbnic: Add FW communication mechanism")
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/ethernet/meta/fbnic/fbnic_fw.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> index edd4adad954a..72c688b17c5b 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
> @@ -127,11 +127,8 @@ static int fbnic_mbx_map_msg(struct fbnic_dev *fbd, int
> mbx_idx,
>  		return -EBUSY;
> 
>  	addr = dma_map_single(fbd->dev, msg, PAGE_SIZE, direction);
> -	if (dma_mapping_error(fbd->dev, addr)) {
> -		free_page((unsigned long)msg);
> -
> +	if (dma_mapping_error(fbd->dev, addr))
>  		return -ENOSPC;
> -	}

Makes sense. It doesn't look like you free_page on other errors either.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> 
>  	mbx->buf_info[tail].msg = msg;
>  	mbx->buf_info[tail].addr = addr;
> --
> 2.49.0

Re: [PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[Paolo Abeni]

On 6/16/25 9:55 PM, Jakub Kicinski wrote:
> The semantics are that caller of fbnic_mbx_map_msg() retains
> the ownership of the message on error. 

FWIW, I think the opposite semantic would lead to simpler/smaller code
overall, but no objections to retain the current one.

/P

Re: [PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[Jakub Kicinski]

On Thu, 19 Jun 2025 12:18:58 +0200 Paolo Abeni wrote:
> On 6/16/25 9:55 PM, Jakub Kicinski wrote:
> > The semantics are that caller of fbnic_mbx_map_msg() retains
> > the ownership of the message on error.   
> 
> FWIW, I think the opposite semantic would lead to simpler/smaller code
> overall, but no objections to retain the current one.

Interesting, I'd have thought the semantics of "on error caller retains
the ownership of the object they are trying to hand over" are generally
considered more intuitive. 

Re: [PATCH net] eth: fbnic: avoid double free when failing to DMA-map FW msg

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 16 Jun 2025 12:55:10 -0700 you wrote:
> The semantics are that caller of fbnic_mbx_map_msg() retains
> the ownership of the message on error. All existing callers
> dutifully free the page.
> 
> Fixes: da3cde08209e ("eth: fbnic: Add FW communication mechanism")
> Reviewed-by: Alexander Duyck <alexanderduyck@fb.com>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> [...]

Here is the summary with links:
  - [net] eth: fbnic: avoid double free when failing to DMA-map FW msg
    https://git.kernel.org/netdev/net/c/5bd1bafd4474

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