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.cb/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.cquoted
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(structseq_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.cb/drivers/net/ethernet/freescale/enetc/enetc4_pf.cquoted
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(structenetc_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(structenetc_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); }