Re: [PATCH net] net: vertexcom: mse102x: Fix possible double free of TX skb

Am 29.10.24 um 23:01 schrieb Jakub Kicinski:

On Tue, 29 Oct 2024 22:15:15 +0100 Stefan Wahren wrote:

quoted

quoted

Isn't it easier to change this function to free the copy rather than
the original? That way the original will remain valid for the callers.

You mean something like this?

diff --git a/drivers/net/ethernet/vertexcom/mse102x.c

b/drivers/net/ethernet/vertexcom/mse102x.c
index a04d4073def9..2c37957478fb 100644

--- a/drivers/net/ethernet/vertexcom/mse102x.c
+++ b/drivers/net/ethernet/vertexcom/mse102x.c

@@ -222,7 +222,7 @@ static int mse102x_tx_frame_spi(struct mse102x_net

*mse, struct sk_buff *txp,
       struct mse102x_net_spi *mses = to_mse102x_spi(mse);
       struct spi_transfer *xfer = &mses->spi_xfer;
       struct spi_message *msg = &mses->spi_msg;
-    struct sk_buff *tskb;
+    struct sk_buff *tskb = NULL;
       int ret;

       netif_dbg(mse, tx_queued, mse->ndev, "%s: skb %p, %d@%p\n",

@@ -235,7 +235,6 @@ static int mse102x_tx_frame_spi(struct mse102x_net

*mse, struct sk_buff *txp,
           if (!tskb)
               return -ENOMEM;

-        dev_kfree_skb(txp);
           txp = tskb;
       }

@@ -257,6 +256,8 @@ static int mse102x_tx_frame_spi(struct mse102x_net

*mse, struct sk_buff *txp,
           mse->stats.xfer_err++;
       }

+    dev_kfree_skb(tskb);
+
       return ret;
   }

Exactly, I think it would work and it feels simpler.

I didn't test it yet, i need access to evaluation board before. But this
change will behave differently regarding stats of tx_bytes [1]. The
first version will include the padding, while the second does not.

[1] -
https://elixir.bootlin.com/linux/v6.12-rc5/source/drivers/net/ethernet/vertexcom/mse102x.c#L445