Thread (1 message) 1 message, 1 author, 6h ago

RE: [PATCH v3 net-next 12/14] net: enetc: remove redundant num_vsi field from enetc_port_caps

From: Wei Fang (OSS) <hidden>
Date: 2026-07-06 02:27:18
Also in: imx, sashiko-reviews

quoted
diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
b/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
quoted
index be378bf8f74d1..5029038bf99fa 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
[ ... ]
quoted
@@ -28,17 +28,14 @@ static void enetc_show_si_mac_hash_filter(struct
seq_file *s, int i)
quoted
 static int enetc_mac_filter_show(struct seq_file *s, void *data)
 {
-	struct enetc_si *si = s->private;
-	struct enetc_hw *hw = &si->hw;
+	struct enetc_pf *pf = enetc_si_priv(s->private);
+	struct enetc_hw *hw = &pf->si->hw;
+	int num_si = pf->total_vfs + 1;
[Severity: Medium]
Could the unconstrained num_si derived from pf->total_vfs cause undefined
behavior in the loops further down in this function?

The original code used caps.num_vsi, which was structurally capped by a 4-bit
hardware register mask. The value total_vfs is read from
pci_sriov_get_totalvfs(), which returns an unvalidated 16-bit PCIe
configuration value.

An arbitrarily large total_vfs from a misconfigured or malicious firmware would
cause PSIPMMR_SI_MAC_MP(i) (which evaluates to BIT(i + 16)) to shift beyond
the bounds of its type when evaluating:
str_enabled_disabled(PSIPMMR_SI_MAC_MP(i) & val).
Actually, the value of ECAPR1[NUM_VSI] field is equal to the value of
PCI_SRIOV_TOTAL_VF register. They are all determined by IERB register
EaVFRIDAR[NUM_VF], which is 4-bit field.

When CONFIG_PCI_IOV is disabled, pf->total_vfs is 0, all hardware resources can
be assigned to PF. When CONFIG_PCI_IOV is enabled, pf->pf->total_vfs is actually
obtained from the PCI_SRIOV_TOTAL_VF register, while the ENETC hardware only
supports a maximum of 15 VFs. And for ENETC, PCI_SRIOV_TOTAL_VF is a read
only register.
quoted
 	struct maft_entry_data maft;
 	struct ntmp_user *user;
-	struct enetc_pf *pf;
 	u32 val, entry_id;
-	int i, num_si;
 	int err = 0;
-
-	pf = enetc_si_priv(si);
-	num_si = pf->caps.num_vsi + 1;
+	int i;
[ ... ]
quoted
diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
quoted
index cab79f81d6fe0..8d536a3831e3a 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
[ ... ]
quoted
@@ -255,34 +254,35 @@ static void enetc4_default_rings_allocation(struct
enetc_pf *pf)
quoted
 {
 	struct enetc_hw *hw = &pf->si->hw;
 	u32 num_rx_bdr, num_tx_bdr, val;
+	int num_vfs = pf->total_vfs;
 	u32 vf_tx_bdr, vf_rx_bdr;
 	int i, rx_rem, tx_rem;
[ ... ]
quoted
-	for (i = 0; i < pf->caps.num_vsi; i++) {
+	for (i = 0; i < num_vfs; i++) {
 		vf_tx_bdr = (i < tx_rem) ? num_tx_bdr + 1 : num_tx_bdr;
 		vf_rx_bdr = (i < rx_rem) ? num_rx_bdr + 1 : num_rx_bdr;
 		val = enetc4_psicfgr0_val_construct(true, vf_tx_bdr, vf_rx_bdr);
-		enetc_port_wr(hw, ENETC4_PSICFGR0(i + 1), val);
+		enetc_port_wr(hw, ENETC4_PSICFGR0(i + 1), val);
[Severity: Medium]
Could this result in out-of-bounds MMIO accesses if a misconfigured or malicious
PCIe device firmware reports a total_vfs value larger than the actual hardware
capacity (e.g., > 15) in its SR-IOV Extended Capability?

Similar to the debugfs issue above, replacing the hardware-capped
caps.num_vsi
with an unvalidated total_vfs causes this unconstrained loop to write past the
instantiated ENETC4_PSICFGR0 register bounds.
For ENETC, PCI_SRIOV_TOTAL_VF is a read only register. Its value is derived
from IERB register EaVFRIDAR[NUM_VF], which is 4-bit field. So I do not think
the total_vfs will be > 15.
quoted
 	}
 }
[ ... ]
quoted
@@ -299,26 +299,25 @@ static void enetc4_allocate_si_rings(struct
enetc_pf *pf)
quoted
 static void enetc4_set_si_msix_num(struct enetc_pf *pf)
 {
 	struct enetc_hw *hw = &pf->si->hw;
-	int i, num_msix, total_si;
+	int num_si = pf->total_vfs + 1;
+	int i, num_msix;
 	u32 val;

-	total_si = pf->caps.num_vsi + 1;
-
-	num_msix = pf->caps.num_msix / total_si +
-		   pf->caps.num_msix % total_si - 1;
+	num_msix = pf->caps.num_msix / num_si +
+		   pf->caps.num_msix % num_si - 1;
 	val = num_msix & PSICFGR2_NUM_MSIX;
 	enetc_port_wr(hw, ENETC4_PSICFGR2(0), val);
[Severity: High]
Can this cause bitwise truncation and starve the PF of interrupts when SR-IOV
is disabled?

If CONFIG_PCI_IOV is disabled, pf->total_vfs evaluates to 0, making
num_si = 1. The driver will then attempt to allocate all available hardware
MSI-X vectors to the PF (num_msix = pf->caps.num_msix - 1).

If the hardware supports more than 64 MSI-X vectors, num_msix will exceed the
6-bit width (max 63) of the hardware register field PSICFGR2_NUM_MSIX.
The bitwise AND operator & PSICFGR2_NUM_MSIX truncates the value instead
of
clamping it (e.g., 64 & 0x3F == 0).

This programs the PF to receive 0 vectors (effectively 1 vector due to the N-1
encoding), starving it of necessary interrupts. The previous implementation
divided by pf->caps.num_vsi + 1, which structurally guarded against this
large allocation.
This is not a problem for current SoCs, because current SoCs support a maximum
of 64 MSI-Xs, so there will be no truncation issue. However, if future SoCs support
more than 64 MSI-Xs, this will indeed be an issue, and I will improve this logic to
better support more capable SoCs in the future.
quoted
-	num_msix = pf->caps.num_msix / total_si - 1;
+	num_msix = pf->caps.num_msix / num_si - 1;
 	val = num_msix & PSICFGR2_NUM_MSIX;
-	for (i = 0; i < pf->caps.num_vsi; i++)
+	for (i = 0; i < pf->total_vfs; i++)
 		enetc_port_wr(hw, ENETC4_PSICFGR2(i + 1), val);
 }
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help