[PATCH net 0/6] net: enetc: SR-IOV robustness and security fixes

[PATCH net 0/6] net: enetc: SR-IOV robustness and security fixes

[Wei Fang]

This series hardens the ENETC PF driver's SR-IOV implementation by
fixing multiple security and stability issues in the VF-to-PF mailbox
communication path, identified through code review.

Issues addressed:
  - Missing input validation for VF requests (security)
  - Race conditions in VF state management (stability)
  - Initialization ordering bugs (reliability)
  - Resource cleanup issues (memory safety)
  - Missing ratelimiting in error paths (DoS prevention)

All issues are theoretical (found through static analysis) but represent
real vulnerabilities exploitable by malicious guests or buggy VF drivers.

Wei Fang (6):
  net: enetc: validate VF primary MAC address before configuration
  net: enetc: fix race condition in VF MAC address configuration
  net: enetc: fix use-after-free in mailbox cleanup on interrupt race
  net: enetc: avoid VF->PF mailbox timeout during SR-IOV teardown
  net: enetc: fix initialization order to prevent use of uninitialized
    resources
  net: enetc: add ratelimiting to VF mailbox error messages

 .../net/ethernet/freescale/enetc/enetc_msg.c  | 34 +++++++++----------
 .../net/ethernet/freescale/enetc/enetc_pf.c   | 28 ++++++++++++---
 .../net/ethernet/freescale/enetc/enetc_pf.h   |  1 +
 3 files changed, 40 insertions(+), 23 deletions(-)

-- 
2.34.1

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

[Wei Fang]

Sashiko reported that the PF driver accepts arbitrary MAC address from
from VF mailbox messages without proper validation, creating a security
vulnerability [1].

In enetc_msg_pf_set_vf_primary_mac_addr(), the MAC address is extracted
directly from the message buffer (cmd->mac.sa_data) and programmed into
hardware via pf->ops->set_si_primary_mac() without any validity checks.
A malicious VF can configure a multicast, broadcast, or all-zero MAC
address.

Therefore, add validation to check the MAC address provided by VF before
configuring it through the mailbox interface. When a VF attempts to set
its primary MAC address via ENETC_MSG_CMD_MNG_ADD command, the PF should
validate whether the provided MAC address is valid.

Reject invalid MAC addresses and return failure status to the VF, with a
rate-limited warning message to prevent log flooding in case of repeated
invalid requests.

This prevents VFs from configuring invalid MAC addresses that could cause
network connectivity issues or unexpected behavior.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index a12fd54a475f..756614ffa8c6 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)) {
+		dev_warn_ratelimited(dev, "VF%d attempted to set invalid MAC",
+				     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);
-- 
2.34.1

[PATCH net 2/6] net: enetc: fix race condition in VF MAC address configuration

[Wei Fang]

Sashiko reported a potential race condition between the VF message
handler and administrative VF MAC configuration from the host [1].

The VF message handler (enetc_msg_pf_set_vf_primary_mac_addr) runs
asynchronously in a workqueue context and accesses vf_state->flags
without any locking. Concurrently, the host can administratively
change the VF MAC address via enetc_pf_set_vf_mac(), which executes
under RTNL lock and modifies both vf_state->flags and hardware
registers.

This creates two race windows:

1) TOCTOU race on vf_state->flags: The check of ENETC_VF_FLAG_PF_SET_MAC
   and subsequent MAC programming are not atomic, allowing the flag state
   to change between check and use.

2) Torn MAC address writes: Hardware MAC programming requires multiple
   non-atomic register writes (__raw_writel for lower 32 bits and
   __raw_writew for upper 16 bits). Concurrent updates from VF mailbox
   and PF admin paths can interleave these operations, resulting in a
   corrupted MAC address being programmed into the hardware.

Fix by introducing a per-VF mutex to serialize access to vf_state and
hardware MAC register updates. Both enetc_pf_set_vf_mac() and
enetc_msg_pf_set_vf_primary_mac_addr() now acquire this lock before
accessing vf_state->flags or programming the MAC address, ensuring
atomic read-modify-write sequences and preventing register write
interleaving.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 10 ++++++++++
 drivers/net/ethernet/freescale/enetc/enetc_pf.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 756614ffa8c6..6939ab04dcdc 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -252,8 +252,12 @@ static int enetc_pf_set_vf_mac(struct net_device *ndev, int vf, u8 *mac)
 		return -EADDRNOTAVAIL;
 
 	vf_state = &pf->vf_state[vf];
+
+	mutex_lock(&vf_state->lock);
 	vf_state->flags |= ENETC_VF_FLAG_PF_SET_MAC;
 	enetc_pf_set_primary_mac_addr(&priv->si->hw, vf + 1, mac);
+	mutex_unlock(&vf_state->lock);
+
 	return 0;
 }
 
@@ -499,12 +503,15 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
 		return ENETC_MSG_CMD_STATUS_FAIL;
 	}
 
+	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);
 	else
 		enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);
 
+	mutex_unlock(&vf_state->lock);
+
 	return ENETC_MSG_CMD_STATUS_OK;
 }
 
@@ -968,6 +975,9 @@ static int enetc_pf_probe(struct pci_dev *pdev,
 					    pf->total_vfs);
 		if (!pf->vf_state)
 			goto err_alloc_vf_state;
+
+		for (int i = 0; i < pf->total_vfs; i++)
+			mutex_init(&pf->vf_state[i].lock);
 	}
 
 	err = enetc_setup_mac_addresses(node, pf);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index ae407e9e9ee7..35d484858c7b 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -14,6 +14,7 @@ enum enetc_vf_flags {
 };
 
 struct enetc_vf_state {
+	struct mutex lock; /* Prevent concurrent access */
 	enum enetc_vf_flags flags;
 };
 
-- 
2.34.1

[PATCH net 3/6] net: enetc: fix use-after-free in mailbox cleanup on interrupt race

[Wei Fang]

Sashiko reported a use-after-free issue in the PF mailbox teardown path
due to incorrect shutdown ordering [1].

In enetc_msg_psi_free(), cancel_work_sync() is called before disabling
the hardware MR interrupt and unregistering the interrupt handler via
free_irq(). This creates a race window where:

1. cancel_work_sync() completes and returns
2. A hardware interrupt arrives and enetc_msg_psi_msix() executes
3. The interrupt handler calls schedule_work(&pf->msg_task),
   re-queuing the work
4. enetc_msg_free_mbx() frees the mailbox DMA buffers
5. The re-queued msg_task work runs and accesses freed memory

The timeline of the race:

  CPU0 (teardown)                   CPU1 (interrupt)
  ================                  ================
  cancel_work_sync()
    -> work cancelled
                                    <interrupt>
                                    enetc_msg_psi_msix()
                                      schedule_work() // work re-queued!
  enetc_msg_disable_mr_int()
  enetc_msg_free_mbx()
    -> DMA buffers freed
  free_irq()
                                    worker runs
                                    -> UAF: access freed rxmsg[]

Fix by reordering the teardown sequence to follow proper driver shutdown
discipline:

1. Synchronously unregister interrupt handler (free_irq)
   - Waits for any running handler to complete
   - Guarantees no more schedule_work() calls after this point
2. Cancel pending work (cancel_work_sync)
   - Now safe, as no new work can be queued
3. Free mailbox resources (enetc_msg_free_mbx)
   - All accessors have stopped
4. Disable hardware interrupt source (enetc_msg_disable_mr_int)
   - Clean up hardware state (defense in depth)

After free_irq() returns, the interrupt handler cannot run anymore,
eliminating the possibility of work re-queuing after cancellation. This
ensures mailbox buffers are only freed after all potential accessors
have been shut down.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_msg.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
index 40d22ebe9224..fe9680a97b0e 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
@@ -151,14 +151,14 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
 	struct enetc_si *si = pf->si;
 	int i;
 
-	cancel_work_sync(&pf->msg_task);
+	/* de-register message passing interrupt handler */
+	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
 
-	/* disable MR interrupts */
-	enetc_msg_disable_mr_int(&si->hw);
+	cancel_work_sync(&pf->msg_task);
 
 	for (i = 0; i < pf->num_vfs; i++)
 		enetc_msg_free_mbx(si, i);
 
-	/* de-register message passing interrupt handler */
-	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
+	/* disable MR interrupts */
+	enetc_msg_disable_mr_int(&si->hw);
 }
-- 
2.34.1

[PATCH net 4/6] net: enetc: avoid VF->PF mailbox timeout during SR-IOV teardown

[Wei Fang]

During SR-IOV teardown, enetc_msg_psi_free() disables the MR interrupt
before pci_disable_sriov() removes the VFs. If a VF sends a mailbox
message during this window, the PF cannot receive it, causing the VF to
timeout waiting for a reply.

Since the timeout occurs during SR-IOV teardown when the VF is about to
be removed anyway, it has no functional impact on operation. However,
more messages will be added in the future, some visible error logs may
confuse users. So fix it by calling pci_disable_sriov() first to remove
all VFs, then safely clean up the mailbox resources. This eliminates the
race window where VFs could send messages to an unresponsive PF.

Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 6939ab04dcdc..052cab48df51 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -544,9 +544,9 @@ static int enetc_sriov_configure(struct pci_dev *pdev, int num_vfs)
 	int err;
 
 	if (!num_vfs) {
+		pci_disable_sriov(pdev);
 		enetc_msg_psi_free(pf);
 		pf->num_vfs = 0;
-		pci_disable_sriov(pdev);
 	} else {
 		pf->num_vfs = num_vfs;
 
-- 
2.34.1

[PATCH net 5/6] net: enetc: fix initialization order to prevent use of uninitialized resources

[Wei Fang]

Sashiko reported a potential issue in enetc_msg_psi_init() where the IRQ
handler is registered before DMA resources are fully initialized [1].

The current initialization sequence is:

  1. request_irq(enetc_msg_psi_msix)    <- IRQ handler registered
  2. INIT_WORK(&pf->msg_task, ...)      <- work_struct initialized
  3. enetc_msg_alloc_mbx()              <- mailbox DMA allocated

This ordering is unsafe because if a spurious interrupt or pending
interrupt from a previous device state fires immediately after
request_irq() returns, the registered ISR enetc_msg_psi_msix() will
execute and unconditionally call:

  schedule_work(&pf->msg_task)

At this point, pf->msg_task has not been initialized by INIT_WORK(), so
the work_struct contains garbage values in its internal linked list
pointers (work_struct->entry). Passing an uninitialized work_struct to
schedule_work() could corrupt the kernel's workqueue linked lists,
potentially leading to:

  - Kernel panic in __queue_work()
  - Memory corruption in workqueue data structures
  - System deadlock or undefined behavior

Additionally, even if the work_struct were initialized, the mailbox DMA
buffers (pf->rxmsg[]) may not yet be allocated when the work handler
enetc_msg_task() runs, resulting in NULL pointer dereference.

Fix by reordering the initialization sequence to ensure all resources are
properly initialized before the interrupt handler can execute:

  1. INIT_WORK(&pf->msg_task, ...)      <- Initialize work first
  2. enetc_msg_alloc_mbx()              <- Allocate all mailboxes
  3. request_irq(enetc_msg_psi_msix)    <- Register IRQ last
  4. Configure hardware & enable MR interrupts

This guarantees that when enetc_msg_psi_msix() runs:
  - pf->msg_task is properly initialized (safe for schedule_work)
  - pf->rxmsg[] buffers are allocated (safe for work handler access)
  - Hardware is configured appropriately

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 .../net/ethernet/freescale/enetc/enetc_msg.c  | 24 +++++++++----------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
index fe9680a97b0e..c156a566a751 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
@@ -109,6 +109,15 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
 	struct enetc_si *si = pf->si;
 	int vector, i, err;
 
+	/* initialize PSI mailbox */
+	INIT_WORK(&pf->msg_task, enetc_msg_task);
+
+	for (i = 0; i < pf->num_vfs; i++) {
+		err = enetc_msg_alloc_mbx(si, i);
+		if (err)
+			goto free_mbx;
+	}
+
 	/* register message passing interrupt handler */
 	snprintf(pf->msg_int_name, sizeof(pf->msg_int_name), "%s-vfmsg",
 		 si->ndev->name);
@@ -117,32 +126,21 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
 	if (err) {
 		dev_err(&si->pdev->dev,
 			"PSI messaging: request_irq() failed!\n");
-		return err;
+		goto free_mbx;
 	}
 
 	/* set one IRQ entry for PSI message receive notification (SI int) */
 	enetc_wr(&si->hw, ENETC_SIMSIVR, ENETC_SI_INT_IDX);
 
-	/* initialize PSI mailbox */
-	INIT_WORK(&pf->msg_task, enetc_msg_task);
-
-	for (i = 0; i < pf->num_vfs; i++) {
-		err = enetc_msg_alloc_mbx(si, i);
-		if (err)
-			goto err_init_mbx;
-	}
-
 	/* enable MR interrupts */
 	enetc_msg_enable_mr_int(&si->hw);
 
 	return 0;
 
-err_init_mbx:
+free_mbx:
 	for (i--; i >= 0; i--)
 		enetc_msg_free_mbx(si, i);
 
-	free_irq(vector, si);
-
 	return err;
 }
 
-- 
2.34.1

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

[Wei Fang]

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].

Fix by replacing dev_err()/dev_warn() with dev_err_ratelimited() and
dev_warn_ratelimited(), limiting output to the default kernel ratelimit.
This ensures errors are still logged for debugging while preventing log
flooding attacks.

Link: https://sashiko.dev/#/patchset/20260511080805.2052495-1-wei.fang%40nxp.com #1
Fixes: beb74ac878c8 ("enetc: Add vf to pf messaging support")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc_pf.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 052cab48df51..73a899b00308 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);
 
@@ -531,8 +532,9 @@ void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int vf_id, u16 *status)
 		*status = enetc_msg_pf_set_vf_primary_mac_addr(pf, vf_id);
 		break;
 	default:
-		dev_err(dev, "command not supported (cmd_type: 0x%x)\n",
-			cmd_type);
+		dev_err_ratelimited(dev,
+				    "command not supported (cmd_type: 0x%x)\n",
+				    cmd_type);
 	}
 }
 
-- 
2.34.1

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

[sashiko-bot]

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

Re: [PATCH net 2/6] net: enetc: fix race condition in VF MAC address configuration

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing return value assignment in enetc_pf_probe() on memory allocation failure leads to false probe success and a later kernel panic.
- [High] Endianness portability bug: Hardware MAC address programming uses native-endian __raw_writel and lacks memory barriers.
- [High] A Time-of-Check to Time-of-Use (TOCTOU) vulnerability exists when validating the VF MAC address from shared DMA memory.
- [Medium] Unbalanced lifecycle: The dynamically allocated vf_state->lock mutexes are never destroyed.
--

commit 1c1cbdf4d36cff2b27997a549d795686b791e573
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: fix race condition in VF MAC address configuration

The VF message handler runs asynchronously and accesses vf_state->flags
without locking. Concurrently, the host can change the VF MAC address
via enetc_pf_set_vf_mac() under RTNL lock, creating TOCTOU races and
torn MAC address writes. Fix this by introducing a per-VF mutex to
serialize access.

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
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -252,8 +252,12 @@ static int enetc_pf_set_vf_mac(struct net_device *ndev, int vf, u8 *mac)
>  		return -EADDRNOTAVAIL;
>  
>  	vf_state = &pf->vf_state[vf];
> +
> +	mutex_lock(&vf_state->lock);
>  	vf_state->flags |= ENETC_VF_FLAG_PF_SET_MAC;
>  	enetc_pf_set_primary_mac_addr(&priv->si->hw, vf + 1, mac);

Is it safe to use __raw_writel and __raw_writew when programming the hardware
MAC address?

Looking at enetc_pf_set_primary_mac_addr():

static void enetc_pf_set_primary_mac_addr(struct enetc_hw *hw, int si,
					  const u8 *addr)
{
	u32 upper = get_unaligned_le32(addr);
	u16 lower = get_unaligned_le16(addr + 4);

	__raw_writel(upper, hw->port + ENETC_PSIPMAR0(si));
	__raw_writew(lower, hw->port + ENETC_PSIPMAR1(si));
}

On big-endian architectures, these native writes lack byte-swapping, which
might result in the hardware register receiving the MAC address bytes in
reverse order compared to little-endian systems. They also lack memory
barriers, which could allow the CPU to reorder the writes.

Could this use the standard writel and writew accessors instead to ensure
proper CPU-to-device endianness conversion?

> +	mutex_unlock(&vf_state->lock);
> +
>  	return 0;
>  }

[ ... ]

> @@ -499,12 +503,15 @@ static u16 enetc_msg_pf_set_vf_primary_mac_addr(struct enetc_pf *pf,
>  		return ENETC_MSG_CMD_STATUS_FAIL;
>  	}
>  
> +	mutex_lock(&vf_state->lock);

Does the addition of this mutex widen a race condition where the VF could
modify the MAC address?

Earlier in enetc_msg_pf_set_vf_primary_mac_addr(), the addr pointer is set
to cmd->mac.sa_data, which points into the message buffer msg->vaddr.

If msg->vaddr is in coherent DMA memory accessible by the VF, the PF driver
validates the MAC address before taking the mutex, but reads it again inside
enetc_pf_set_primary_mac_addr() after taking the mutex.

Could a malicious VF exploit this double-fetch by writing an invalid MAC
address into the shared DMA buffer right after the validation check passes,
bypassing the check entirely? Might it be safer to copy the message to a
local buffer before validating it?

>  	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);
>  	else
>  		enetc_pf_set_primary_mac_addr(&pf->si->hw, vf_id + 1, addr);
>  
> +	mutex_unlock(&vf_state->lock);
> +
>  	return ENETC_MSG_CMD_STATUS_OK;
>  }

[ ... ]

> @@ -968,6 +975,9 @@ static int enetc_pf_probe(struct pci_dev *pdev,
>  					    pf->total_vfs);
>  		if (!pf->vf_state)
>  			goto err_alloc_vf_state;

If the allocation for pf->vf_state fails here, err does not appear to be
assigned a failure code before the jump.

Since err would still hold 0 from the earlier successful call to
enetc_pf_register_with_ierb(), could this cause the probe function to falsely
return success to the PCI subsystem?

If so, this might lead to a kernel panic later when enetc_pf_remove() attempts
to dereference the unallocated device structure.

> +
> +		for (int i = 0; i < pf->total_vfs; i++)
> +			mutex_init(&pf->vf_state[i].lock);

Is it necessary to call mutex_destroy() for these dynamically allocated
mutexes?

If kfree(pf->vf_state) is called during the probe error path or during
device removal, freeing the active mutex memory without destroying it first
might cause lockdep to report use-after-free or list corruption warnings
when debugging options are enabled.

>  	}
>  
>  	err = enetc_setup_mac_addresses(node, pf);

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

Re: [PATCH net 3/6] net: enetc: fix use-after-free in mailbox cleanup on interrupt race

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Reordering teardown leaves hardware interrupts enabled after unregistering the OS handler, causing pending MSI-X interrupts that crash the kernel on re-initialization.
- [High] DMA memory Use-After-Free in mailbox cleanup because buffers are freed before clearing hardware receive registers.
- [High] Initialization order bug in `enetc_msg_psi_init()` leading to kernel panic on pending interrupts.
--

commit 7775e7cd27db0383dcec0aee22ae84d6c2378ee2
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: fix use-after-free in mailbox cleanup on interrupt race
    
This commit fixes a use-after-free regression in the PF mailbox teardown path
by reordering the shutdown sequence to prevent work requeuing.

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

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> index 40d22ebe9224f..fe9680a97b0e7 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> @@ -151,14 +151,14 @@ void enetc_msg_psi_free(struct enetc_pf *pf)
>  	struct enetc_si *si = pf->si;
>  	int i;
>  
> -	cancel_work_sync(&pf->msg_task);
> +	/* de-register message passing interrupt handler */
> +	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
>  
> -	/* disable MR interrupts */
> -	enetc_msg_disable_mr_int(&si->hw);
> +	cancel_work_sync(&pf->msg_task);

Could moving free_irq() before enetc_msg_disable_mr_int() leave hardware
interrupts enabled after the OS handler is unregistered?

Because VFs are still active at this stage, a VF can send a message.
cancel_work_sync() waits for the worker, which processes existing messages
but then unconditionally re-arms the MR interrupt before exiting.

If a VF sends a message now, the hardware asserts the interrupt. Since
the OS handler is removed and the vector is masked, it is recorded as pending
in the MSI-X Pending Bit Array. enetc_msg_disable_mr_int() is called later
but does not clear this array.

Upon the next SR-IOV enablement, request_irq() unmasks the vector, and
the pending interrupt fires immediately, leading to a crash.

This regression appears to be mitigated later in the patch series by
commit 20bd14a48fb81 ("net: enetc: avoid VF->PF mailbox timeout during
SR-IOV teardown") and commit fa9d7e3799c54 ("net: enetc: fix initialization
order to prevent use of uninitialized resources").

>  	for (i = 0; i < pf->num_vfs; i++)
>  		enetc_msg_free_mbx(si, i);

This isn't a regression introduced by this patch, but does the current
implementation of enetc_msg_free_mbx() have a use-after-free risk for the
mailbox memory?

It calls dma_free_coherent() to free the mailbox memory before clearing the
hardware DMA address registers:

enetc_msg_free_mbx() {
	...
	dma_free_coherent(&si->pdev->dev, msg->size, msg->vaddr, msg->dma);
	memset(msg, 0, sizeof(*msg));

	enetc_wr(hw, ENETC_PSIVMSGRCVAR0(idx), 0);
	enetc_wr(hw, ENETC_PSIVMSGRCVAR1(idx), 0);
}

Since VFs are still active during SR-IOV teardown, a VF could send a message
during this window. This would cause the PF hardware to perform a DMA write
using the uncleared receive address into the just-freed memory.

This regression appears to be mitigated later in the patch series by
commit 20bd14a48fb81 ("net: enetc: avoid VF->PF mailbox timeout during
SR-IOV teardown"), which disables SR-IOV before mailbox cleanup.

> -	/* de-register message passing interrupt handler */
> -	free_irq(pci_irq_vector(si->pdev, ENETC_SI_INT_IDX), si);
> +	/* disable MR interrupts */
> +	enetc_msg_disable_mr_int(&si->hw);
>  }

Additionally, while not introduced by this patch, is there an initialization
order regression in enetc_msg_psi_init()?

If request_irq() is called to register enetc_msg_psi_msix() before INIT_WORK()
and enetc_msg_alloc_mbx() are executed, an immediate interrupt could cause the
handler to call schedule_work() on an uninitialized work structure or lead
to a NULL pointer dereference.

This regression appears to be fixed later in the patch series by
commit fa9d7e3799c54 ("net: enetc: fix initialization order to prevent use
of uninitialized resources").

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

Re: [PATCH net 4/6] net: enetc: avoid VF->PF mailbox timeout during SR-IOV teardown

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Missing bounds checks on the number of VFs against the fixed-size rxmsg array can lead to out-of-bounds heap reads/writes and out-of-bounds MMIO accesses.
- [High] Missing cancel_work_sync() in the error path of enetc_msg_psi_init() leads to a potential NULL pointer dereference or unhandled interrupt.
--

commit 20bd14a48fb81c18f7cc12a0a4f0d93f01026fce
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: avoid VF->PF mailbox timeout during SR-IOV teardown

During SR-IOV teardown, this commit reorders the cleanup steps to call
pci_disable_sriov() before cleaning up the mailbox resources. This
eliminates a race window where VFs could send messages to an unresponsive
PF and time out waiting for a reply.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> index 6939ab04dcdc0..052cab48df514 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -544,9 +544,9 @@ static int enetc_sriov_configure(struct pci_dev *pdev, int num_vfs)
>  	int err;
>  
>  	if (!num_vfs) {
> +		pci_disable_sriov(pdev);
>  		enetc_msg_psi_free(pf);
>  		pf->num_vfs = 0;
> -		pci_disable_sriov(pdev);
>  	} else {
>  		pf->num_vfs = num_vfs;

This problem wasn't introduced by this patch, but it exists in the codebase.
In enetc_msg_psi_init(), if enetc_msg_alloc_mbx() fails:

drivers/net/ethernet/freescale/enetc/enetc_msg.c:enetc_msg_psi_init() {
[ ... ]
	for (i = 0; i < pf->num_vfs; i++) {
		err = enetc_msg_alloc_mbx(si, i);
		if (err)
			goto err_init_mbx;
	}
[ ... ]
err_init_mbx:
	for (i--; i >= 0; i--)
		enetc_msg_free_mbx(si, i);

	free_irq(vector, si);

	return err;
}

Can this lead to a NULL pointer dereference or an unhandled interrupt?

If an interrupt fired and scheduled the work item before free_irq() was
called, would the work item remain queued and run asynchronously after the
function returns, since cancel_work_sync(&pf->msg_task) is missing?

A prior patch in the series fixed the cleanup in the normal teardown path,
but does this error path remain vulnerable?

If the executing enetc_msg_task() then runs, would it dereference the
mailbox memory (msg->vaddr) that was just cleared if a message was pending?
Would it also call enetc_msg_enable_mr_int(), enabling hardware interrupts
when no handler is registered anymore?


There is another problem that wasn't introduced by this patch, but exists
in the codebase. During enetc_pf_probe(), the driver initializes
pf->total_vfs:

drivers/net/ethernet/freescale/enetc/enetc_pf.c:enetc_pf_probe() {
[ ... ]
	pf->total_vfs = pci_sriov_get_totalvfs(pdev);
	if (pf->total_vfs) {
		pf->vf_state = kzalloc_objs(struct enetc_vf_state,
					    pf->total_vfs);
[ ... ]
}

Does this code lack bounds checking on the number of VFs against the fixed
size of the rxmsg array (ENETC_MAX_NUM_VFS)?

If a PCIe device or hypervisor presents a capability with an artificially
large TotalVFs, would loops iterating up to pf->num_vfs perform out-of-bounds
accesses to the pf->rxmsg array when SR-IOV is enabled?

Could this also result in out-of-bounds MMIO writes by indexing past
valid registers?

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

Re: [PATCH net 5/6] net: enetc: fix initialization order to prevent use of uninitialized resources

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] This problem wasn't introduced by this patch, but lack of bounds checking on `pf->num_vfs` leads to an out-of-bounds array access in `enetc_msg_alloc_mbx()`.
- [High] This problem wasn't introduced by this patch, but in `enetc_msg_free_mbx()`, `dma_free_coherent()` is called to free the mailbox DMA buffer before writing 0 to the device's DMA address registers (`ENETC_PSIVMSGRCVAR0/1`).
- [High] This problem wasn't introduced by this patch, but stale hardware state in the Message Receive Register (`ENETC_PSIMSGRR`) can cause an infinite loop in the `enetc_msg_task()` workqueue handler.
- [High] This problem wasn't introduced by this patch, but lockless read-modify-write operations on the `ENETC_PSIIER` register can clobber interrupt states.
--

commit fa9d7e3799c5420c70515e10bd1626738a874ff5
Author: Wei Fang <wei.fang@nxp.com>
net: enetc: fix initialization order to prevent use of uninitialized resources

This commit reorders the initialization sequence in enetc_msg_psi_init() to
ensure all resources, including the msg_task work struct and mailbox DMA
buffers, are fully initialized before the interrupt handler is registered.
This prevents spurious interrupts from causing kernel panics or memory
corruption by scheduling an uninitialized work struct.

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

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_msg.c b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> index fe9680a97b0e7..c156a566a751e 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_msg.c
> @@ -109,6 +109,15 @@ int enetc_msg_psi_init(struct enetc_pf *pf)
>  	struct enetc_si *si = pf->si;
>  	int vector, i, err;
>  
> +	/* initialize PSI mailbox */
> +	INIT_WORK(&pf->msg_task, enetc_msg_task);

This problem wasn't introduced by this patch, but can stale hardware state in
the Message Receive Register (ENETC_PSIMSGRR) cause an infinite loop in the
enetc_msg_task() workqueue handler?

If the device was previously configured with more VFs, and a bit for a higher
VF remains set in ENETC_PSIMSGRR, re-initializing the driver with fewer VFs
might trigger a deadlock.

The inner loop in enetc_msg_task() only iterates up to the current pf->num_vfs
and therefore ignores the stale bit. Because the bit is never cleared
(it is a w1c register), the outer for (;;) loop continuously reads a non-zero
mr_mask and spins indefinitely, locking up the kernel worker thread.

> +
> +	for (i = 0; i < pf->num_vfs; i++) {
> +		err = enetc_msg_alloc_mbx(si, i);

This problem wasn't introduced by this patch, but does the lack of bounds
checking on pf->num_vfs lead to an out-of-bounds array access in
enetc_msg_alloc_mbx()?

The array pf->rxmsg has a fixed size of ENETC_MAX_NUM_VFS (2). However,
pf->num_vfs is populated from the user-requested SR-IOV VFs, bounded only by
the hardware's total_VFs capability.

If a device reports total_VFs > 2, a user could request more than 2 VFs,
causing enetc_msg_alloc_mbx() to iterate past the end of the rxmsg array.

This out-of-bounds write would corrupt subsequent fields in struct enetc_pf
(including the msg_task workqueue struct initialized by this patch), leading
to kernel crashes and potential privilege escalation when the corrupted work
struct is scheduled.

> +		if (err)
> +			goto free_mbx;
> +	}
> +

[ ... ]

> @@ -117,32 +126,21 @@ int enetc_msg_psi_init(struct enetc_pf *pf)

[ ... ]

>  	/* enable MR interrupts */
>  	enetc_msg_enable_mr_int(&si->hw);

This problem wasn't introduced by this patch, but can lockless
read-modify-write operations on the ENETC_PSIIER register clobber interrupt
states?

Looking at enetc_msg_enable_mr_int():

drivers/net/ethernet/freescale/enetc/enetc_msg.c:enetc_msg_enable_mr_int() {
	u32 psiier = enetc_rd(hw, ENETC_PSIIER);

	enetc_wr(hw, ENETC_PSIIER, psiier | ENETC_PSIIER_MR_MASK);
}

enetc_msg_enable_mr_int() and enetc_msg_disable_mr_int() perform unprotected
read-modify-write operations on the ENETC_PSIIER register.

Because enetc_msg_enable_mr_int() can run in process context (via the
workqueue or during initialization) and enetc_msg_disable_mr_int() runs in
hard interrupt context (enetc_msg_psi_msix()), a race exists.

If an interrupt fires while the workqueue is executing the RMW operation, the
workqueue could write back a stale value of ENETC_PSIIER, obliterating the
state changes made by the interrupt handler. Should this be protected by a
spinlock?

>  
>  	return 0;
>  
> -err_init_mbx:
> +free_mbx:
>  	for (i--; i >= 0; i--)
>  		enetc_msg_free_mbx(si, i);

This problem wasn't introduced by this patch, but in enetc_msg_free_mbx(), is
the mailbox DMA buffer freed before clearing the device's DMA address
registers?

Looking at enetc_msg_free_mbx():

drivers/net/ethernet/freescale/enetc/enetc_msg.c:enetc_msg_free_mbx() {
	...
	dma_free_coherent(&si->pdev->dev, msg->size, msg->vaddr, msg->dma);
	memset(msg, 0, sizeof(*msg));

	enetc_wr(hw, ENETC_PSIVMSGRCVAR0(idx), 0);
	enetc_wr(hw, ENETC_PSIVMSGRCVAR1(idx), 0);
}

Freeing the DMA memory before clearing the hardware registers allows the
hardware to perform a DMA write into freed kernel memory if a message arrives
or a pending DMA completes in that race window, leading to memory corruption.

Should the hardware DMA configuration be cleared before the backing memory is
freed?

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

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

[sashiko-bot]

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