Re: [PATCH v3 net-next 03/15] net: enetc: add link status message support to PF driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Denial of Service via Synchronous Polling of Untrusted VF
- [High] Logic Error and TOCTOU Race Permanently Dropping Link State Notifications
--

commit 0897c02e67549130fea1a46e3080e9af30c83727
Author: Wei Fang <wei.fang@nxp.com>

    net: enetc: add link status message support to PF driver
    
    Add link status message support to the PF driver using three command IDs
    under message class 0x80 (ENETC_MSG_CLASS_ID_LINK_STATUS).

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> index a89a5a418a23d..e21414acdc0d0 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
[ ... ]
> +static int enetc_pf_send_msg(struct enetc_pf *pf, u32 msg_code, u16 ms_mask)
> +{
> +	struct enetc_hw *hw = &pf->si->hw;
> +	u16 old_ms_mask = ms_mask;
> +	u16 ms_status;
> +	u32 val;
> +
> +	/* The MS bit is set, indicating that the corresponding VF has not
> +	 * read the last message, PF cannot send new message to the VF. To
> +	 * avoid sending messages to such a VF, the bit corresponding to VF
> +	 * is cleared from ms_mask. Because the MS bit can only be written
> +	 * as 1, writing a 0 has no effect. Writing a 1 when the bit is
> +	 * already set is undefined.
> +	 */
> +	ms_status = enetc_rd(hw, ENETC_PSIMSGSR) & 0xffff;
> +	ms_mask &= ~ms_status;
> +	if (!ms_mask)
> +		return -EIO;

[Severity: High]
Does this logic introduce a time-of-check to time-of-use race condition?

If a VF acknowledges a previous message by clearing its MS bit immediately
after ms_status is read, the mask will remain zero and this early return
will prevent the link state from being updated.

Because the early return prevents the shared MC payload from being updated
in hardware, and there is no retry logic, will the VF receive the old link
state when it eventually reads the register and be left permanently out of
sync with the PF?

> +
> +	if (ms_mask != old_ms_mask)
> +		dev_warn_ratelimited(&pf->si->pdev->dev,
> +				     "PF cannot send message to VF(s) 0x%x\n",
> +				     ms_mask ^ old_ms_mask);
> +
> +	enetc_wr(hw, ENETC_PSIMSGSR,
> +		 FIELD_PREP(PSIMSGSR_MC, msg_code) | ms_mask);
> +
> +	return read_poll_timeout(enetc_rd, val, !(val & ms_mask), 1000,
> +				 200000, false, hw, ENETC_PSIMSGSR);

[Severity: High]
Can an untrusted VF use this synchronous polling to intentionally stall the
PF's execution thread?

This blocks for up to 200ms waiting for the VF to acknowledge the message.
Since this is reachable by untrusted VFs (e.g., running in a VM), can a
malicious VF ignore the message and tie up the PF thread?

[ ... ]
> +static void enetc_msg_register_link_status_notifier(struct enetc_pf *pf,
> +						    int vf_id)
> +{
> +	u16 pf_msg = FIELD_PREP(ENETC_PF_MSG_CLASS_ID,
> +				ENETC_MSG_CLASS_ID_CMD_SUCCESS);
> +
> +	mutex_lock(&pf->msg_lock);
> +
> +	enetc_pf_reply_msg(&pf->si->hw, vf_id, pf_msg);
> +
> +	/* SR-IOV is being disabled if pf->sriov_enabled is false, so no
> +	 * need to set link_status_ms_mask and notify the link status.
> +	 */
> +	if (!pf->sriov_enabled)
> +		goto msg_unlock;
> +
> +	pf->link_status_ms_mask |= PSIMSGSR_MS(vf_id);
> +
> +	/* Notify VF the current link status */
> +	enetc_msg_notify_vf_link_status(pf, PSIMSGSR_MS(vf_id));

[Severity: High]
Does calling enetc_msg_notify_vf_link_status() here block other mailbox
operations?

Because enetc_msg_notify_vf_link_status() eventually calls
enetc_pf_send_msg() which can poll for 200ms, and we are holding
pf->msg_lock, will a slow or malicious VF block all other VF mailbox
operations and link state processing for this PF?

> +
> +msg_unlock:
> +	mutex_unlock(&pf->msg_lock);
> +}

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