Re: [PATCH net 6/6] net: enetc: add ratelimiting to VF mailbox error messages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The VF MAC address is validated directly from a shared DMA buffer, allowing a malicious guest VM to bypass validation via a Time-Of-Check to Time-Of-Use (TOCTOU) race condition.
- [High] The unbounded loop in `enetc_msg_task()` allows a malicious guest VM to cause a CPU soft lockup on the host, leading to a Denial of Service.
--

commit 88150cd0493bf6368f09cabbbd2a6aba99ee0cf5
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: add ratelimiting to VF mailbox error messages

This commit replaces dev_err() and dev_warn() with their ratelimited
counterparts in the vf-to-pf messaging paths. The change aims to prevent
a buggy or malicious guest vm from flooding the host kernel log. This
ensures errors remain debuggable while mitigating log flooding attacks.

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

> Sashiko reported that a buggy or malicious guest VM can flood the host
> kernel log by repeatedly sending VF-to-PF messages at a high rate,
> degrading host performance and hiding important system logs [1].

This isn't a regression introduced by this patch, but does the unbounded
loop in enetc_msg_task() still allow a malicious guest vm to cause a cpu
soft lockup on the host?

In enetc_msg_task(), vf messages are processed within an unbounded loop:

drivers/net/ethernet/freescale/enetc/enetc_msg.c:enetc_msg_task() {
    ...
	for (;;) {
		mr_mask = enetc_rd(hw, ENETC_PSIMSGRR) & ENETC_PSIMSGRR_MR_MASK;
		if (!mr_mask) {
    ...
}

If a malicious guest vm continuously sends messages at a high rate, the vf
can instantly send another as soon as the pf acknowledges a message.

Because the worker thread never yields (e.g., via cond_resched()) or enforces
a processing budget, would the mr_mask check frequently evaluate to non-zero,
causing the pf to spin indefinitely?

Could this monopolize the host cpu core running the workqueue, leading to a
watchdog soft lockup and a guest-to-host denial of service?

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 052cab48df514..73a899b00308b 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -505,8 +505,9 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
>  
>  	mutex_lock(&vf_state->lock);
>  	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);
> +		dev_warn_ratelimited(dev,
> +				     "VF%d attempted to override PF set MAC\n",
> +				     vf_id);
>  	else
>  		enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);

This isn't a regression introduced by this patch, but does validating the
mac address directly from the shared dma buffer allow a time-of-check to
time-of-use race condition?

The pointer addr points directly into the msg->vaddr dma buffer. The pf
validates the mac address by calling is_valid_ether_addr(addr) and then passes
the same pointer to enetc_pf_set_primary_mac_addr(), which reads the mac
address again from the dma buffer to write it to the hardware.

Could a malicious vf exploit this by writing a valid mac address to the dma
buffer to pass the validation check, and then overwriting it with an invalid
or multicast mac address before the pf writes it to the hardware registers?

Should the mac address be copied to a local stack buffer before validation?

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