Re: [PATCH net-next 01/15] net: enetc: add trusted VF support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] UAPI Breakage: The patch unconditionally denies MAC address changes for untrusted VFs and fails to implement correct trusted VF override semantics.
--

commit 6f75af6dda440eac57a8d16c81c6cc7d3ceba0b9
Author: Claudiu Manoil <claudiu.manoil@nxp.com>

net: enetc: add trusted VF support

Some mailbox messages require a higher privilege level to be executed
on behalf of the requesting VF. Introduce a trusted VF flag
(ENETC_VF_FLAG_TRUSTED) and wire up the ndo_set_vf_trust callback via
enetc_pf_set_vf_trust().

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> index edc1277bb5860..156333ef26bf3 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
[ ... ]
> @@ -61,31 +63,41 @@ static u16 enetc_msg_set_vf_primary_mac_addr(struct enetc_pf *pf, int vf_id,
>  	struct enetc_vf_state *vf_state = &pf->vf_state[vf_id];
>  	struct enetc_msg_mac_exact_filter *msg = vf_msg;
>  	struct device *dev = &pf->si->pdev->dev;
> +	u16 pf_msg = ENETC_PF_MSG_SUCCESS;
>  	char *addr = msg->mac[0].addr;
>  
> +	mutex_lock(&vf_state->lock);
> +
> +	if (!(vf_state->flags & ENETC_VF_FLAG_TRUSTED)) {
> +		pf_msg = ENETC_PF_MSG_PERM_DENY;
> +		goto vf_state_unlock;
> +	}

[Severity: High]
Under standard Linux SR-IOV semantics, untrusted VFs are allowed to set their
own MAC address provided the PF has not administratively locked it (i.e. when
PF_SET_MAC is false). Does this check unconditionally deny all MAC changes from
untrusted VFs? If so, couldn't this break backward compatibility and break guest
OSes that attempt to assign a random MAC on boot?

> +
>  	if (!is_valid_ether_addr(addr)) {
>  		dev_err_ratelimited(dev, "VF%d attempted to set invalid MAC\n",
>  				    vf_id);
> -		return (FIELD_PREP(ENETC_PF_MSG_CLASS_ID,
> -				   ENETC_MSG_CLASS_ID_MAC_FILTER) |
> -			FIELD_PREP(ENETC_PF_MSG_CLASS_CODE,
> -				   ENETC_MF_CLASS_CODE_INVALID_MAC));
> +		pf_msg = FIELD_PREP(ENETC_PF_MSG_CLASS_ID,
> +				    ENETC_MSG_CLASS_ID_MAC_FILTER) |
> +			 FIELD_PREP(ENETC_PF_MSG_CLASS_CODE,
> +				    ENETC_MF_CLASS_CODE_INVALID_MAC);
> +		goto vf_state_unlock;
>  	}
>  
> -	mutex_lock(&vf_state->lock);
>  	if (vf_state->flags & ENETC_VF_FLAG_PF_SET_MAC) {
> -		mutex_unlock(&vf_state->lock);
>  		dev_err_ratelimited(dev,
>  				    "VF%d attempted to override PF set MAC\n",
>  				    vf_id);
> -		return FIELD_PREP(ENETC_PF_MSG_CLASS_ID,
> -				  ENETC_MSG_CLASS_ID_CMD_NOT_PERMITTED);
> +		pf_msg = FIELD_PREP(ENETC_PF_MSG_CLASS_ID,
> +				    ENETC_MSG_CLASS_ID_CMD_NOT_PERMITTED);
> +		goto vf_state_unlock;
>  	}

[Severity: High]
Does this incorrectly deny MAC changes from trusted VFs if the PF has assigned
a MAC? Since this ENETC_VF_FLAG_PF_SET_MAC check happens after the TRUSTED check
and unconditionally rejects the operation, it appears to violate the standard
semantics that trusted VFs can override a PF-set MAC.

>  
>  	enetc_set_si_hw_addr(pf, vf_id + 1, addr);
> +
> +vf_state_unlock:
>  	mutex_unlock(&vf_state->lock);
>  
> -	return ENETC_PF_MSG_SUCCESS;
> +	return pf_msg;
>  }

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