Re: [PATCH v3 net] net: enetc: fix VSI mailbox timeout handling and DMA lifecycle

[Simon Horman]

From: 'Simon Horman' <horms@xxxxxxxxxx>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
Subject: net: enetc: fix VSI mailbox timeout handling and DMA lifecycle

This commit attempts to fix a use-after-free issue in the VSI mailbox
send path by deferring the freeing of the DMA buffer if the hardware is
still processing the message. The DMA buffer information is stored in the
si structure and freed on the next send or when the driver is removed.

[ ... ]

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c

[ ... ]

> @@ -259,6 +282,7 @@ static void enetc_vf_remove(struct pci_dev *pdev)
>  {
>  	struct enetc_si *si = pci_get_drvdata(pdev);
>  	struct enetc_ndev_priv *priv;
> +	struct enetc_msg_swbd msg;
>  
>  	priv = netdev_priv(si->ndev);
>  	unregister_netdev(si->ndev);
> @@ -270,7 +294,9 @@ static void enetc_vf_remove(struct pci_dev *pdev)
>  
>  	free_netdev(si->ndev);
>  
> +	msg = si->msg;
>  	enetc_pci_remove(pdev);
> +	enetc_msg_dma_free(&pdev->dev, &msg);

Is it possible for the mailbox to still be busy here?

The commit addresses the use-after-free in the send path by checking the
mailbox busy status before freeing the previous buffer. However, during
driver teardown, enetc_msg_dma_free() is called unconditionally.

If a prior message sent via enetc_msg_vsi_send() times out and the hardware
is still accessing the buffer, and then the driver is unbound, could this
cause the hardware to read from freed DMA memory?

Should the teardown path wait for the mailbox to become idle or ensure the
hardware is safely reset before freeing the buffer?

>  }