Re: [PATCH net 1/6] net: enetc: validate VF primary MAC address before configuration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Double fetch (TOCTOU) vulnerability on the MAC address in the DMA mailbox buffer.
- [Low] Missing newline character in the `dev_warn_ratelimited` format string.
--

commit 4e5ebbc802e563f230eda3225f5ff1686a5568c8
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: validate VF primary MAC address before configuration

This commit adds validation to check the mac address provided by the vf via
the mailbox interface. It rejects invalid mac addresses and returns a failure
status to prevent network connectivity issues or unexpected behavior.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index a12fd54a475f6..756614ffa8c6e 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -493,6 +493,12 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
>  		return ENETC_MSG_CMD_STATUS_FAIL;
>  
>  	addr = cmd->mac.sa_data;
> +	if (!is_valid_ether_addr(addr)) {

Does this introduce a double fetch regression?

The addr variable points to cmd->mac.sa_data, which resides within the
msg->vaddr dma-coherent receive buffer shared with the hardware/vf.

The code first reads the mac address from this dma buffer to validate it
using is_valid_ether_addr().

If validation passes, enetc_pf_set_primary_mac_addr() is called with the
same addr pointer, which reads the bytes from the dma buffer a second time
(via get_unaligned_le32() and get_unaligned_le16()) to program the hardware
registers.

Could a malicious vf exploit this window by overwriting the mac address in
the dma buffer immediately after the validation check but before the hardware
is programmed?

If the mac address is copied into a local array (e.g., u8 mac[ETH_ALEN])
before validation and use, would it prevent this issue?

Note that subsequent patches in the series (e.g., commit "net: enetc: fix
race condition in vf mac address configuration") add a mutex to protect
vf_state->flags, but do not copy the dma buffer, so this potential race
with the hardware seems to remain.

> +		dev_warn_ratelimited(dev, "VF%d attempted to set invalid MAC",

This isn't a bug, but should this format string end with a newline character
(\n)?

> +				     vf_id);
> +		return ENETC_MSG_CMD_STATUS_FAIL;
> +	}
> +
>  	if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC)
>  		dev_warn(dev, "Attempt to override PF set mac addr for VF%d\n",
>  			 vf_id);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513103021.2190593-1-wei.fang@nxp.com?part=1