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

On Tue, 22 Oct 2024 17:52:42 +0200 Stefan Wahren wrote:

-static int mse102x_tx_frame_spi(struct mse102x_net *mse, struct sk_buff *txp, +static int mse102x_tx_frame_spi(struct mse102x_net *mse, struct sk_buff **txp, unsigned int pad) { struct mse102x_net_spi *mses = to_mse102x_spi(mse); @@ -226,29 +226,29 @@ static int mse102x_tx_frame_spi(struct mse102x_net *mse, struct sk_buff *txp, int ret;

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

•   __func__, txp, txp->len, txp->data);
  

•   __func__, *txp, (*txp)->len, (*txp)->data);
  
  

• if ((skb_headroom(txp) < DET_SOF_LEN) ||

•    (skb_tailroom(txp) < DET_DFT_LEN + pad)) {
  

• tskb = skb_copy_expand(txp, DET_SOF_LEN, DET_DFT_LEN + pad,
  

• if ((skb_headroom(*txp) < DET_SOF_LEN) ||

•    (skb_tailroom(*txp) < DET_DFT_LEN + pad)) {
  

• tskb = skb_copy_expand(*txp, DET_SOF_LEN, DET_DFT_LEN + pad,
  	       GFP_KERNEL);
  

  if (!tskb) return -ENOMEM;

• dev_kfree_skb(txp);
  

• txp = tskb;
  

• dev_kfree_skb(*txp);
  

• *txp = tskb;
  

  }

• mse102x_push_header(txp);

• mse102x_push_header(*txp);

  if (pad)

• skb_put_zero(txp, pad);
  

• skb_put_zero(*txp, pad);
  
  

• mse102x_put_footer(txp);

• mse102x_put_footer(*txp);

• xfer->tx_buf = txp->data;

• xfer->tx_buf = (*txp)->data; xfer->rx_buf = NULL;

• xfer->len = txp->len;

• xfer->len = (*txp)->len;

  ret = spi_sync(mses->spidev, msg); if (ret < 0) {

@@ -368,7 +368,7 @@ static void mse102x_rx_pkt_spi(struct mse102x_net *mse) mse->ndev->stats.rx_bytes += rxlen;

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.