Message-ID: <a929cf9c-dc65-48aa-bff4-7622d72c983d@intel.com>
Date: Thu, 28 Aug 2025 15:38:38 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: <qianjiaru77@...il.com>, <michael.chan@...adcom.com>,
	<pavan.chebbi@...adcom.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<kuba@...nel.org>, <pabeni@...hat.com>, <andrew+netdev@...n.ch>
CC: <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] RFS Capability Bypass Vulnerability in Linux bnxt_en
 Driver



On 8/26/2025 9:26 AM, qianjiaru77@...il.com wrote:
> From: qianjiaru <qianjiaru77@...il.com>
> 
> A logic vulnerability exists in the `bnxt_rfs_capable()` function of the 
> Linux kernel's bnxt_en network driver. The vulnerability allows the driver
> to incorrectly report RFS (Receive Flow Steering) capability on older 
> firmware versions without performing necessary resource validation, 
> potentially leading to system crashes when RFS features are enabled.
> The vulnerability exists in the RFS capability check logic where older 
> firmware versions bypass essential resource validation. 
> 

Do you have an actual crash report?

> The problematic code is:
> ```c
> // Lines 13594-13595 in drivers/net/ethernet/broadcom/bnxt/bnxt.c
> if (!BNXT_NEW_RM(bp))
>     return true;  // VULNERABLE: Unconditional success for old firmware
> ```
> 
> ##Vulnerability Mechanism:
> 
> 1. **Bypassed Validation**: 
> Old firmware path completely skips resource availability checks
> 2. **False Capability Report**: 
> Function reports RFS as available when resources may be insufficient  
> 3. **Subsequent Failure**: 
> When RFS is actually enabled, insufficient resources
> cause driver/system crashes
> 4. **State Inconsistency**: 
> Driver state doesn't match hardware capabilities
> 
> ##Attack Scenario
> 
> 1. Administrator configures RFS/NTUPLE filtering 
> on device with old firmware
> 2. `bnxt_rfs_capable()` incorrectly returns `true` 
> without resource validation
> 3. Driver attempts to allocate hardware resources 
> that don't exist
> 4. System crash or memory corruption occurs during 
> resource allocation
> 5. Network service disruption affects the entire system
> 
> ## Comparison with Similar Vulnerabilities
> 
> This vulnerability is a **direct variant** of CVE-2024-44933,
> which involved similar firmware version handling issues:
> 
> - **CVE-2024-44933**: 
> RSS table size mismatches due to firmware version differences
> - **This vulnerability**: 
> RFS capability reporting bypasses validation for old firmware
> 
> Both share the same anti-pattern: 
> firmware version-specific code paths 
> with reduced validation for older versions.
> 
> The root cause pattern appears throughout the bnxt driver codebase
> where legacy firmware support introduces security vulnerabilities.
> 
> ##Proposed Fix
> 
> The vulnerability should be fixed by ensuring 
> consistent validation across all firmware versions:
> 
> ```c
> // Current vulnerable code:
> if (!BNXT_NEW_RM(bp))
>     return true;
> 
> // Proposed secure fix:
> if (!BNXT_NEW_RM(bp)) {
>     // Even on old firmware, validate basic resource constraints
>     return (hwr.vnic <= max_vnics && hwr.rss_ctx <= max_rss_ctxs);
> }
> ```
> 
> ## Additional Variant Risks
> 
> This analysis revealed another related vulnerability in the same driver:
> 
> **`bnxt_hwrm_reserve_vf_rings()` function (lines 7782-7785)** 
> contains a similar pattern:
> ```c
> if (!BNXT_NEW_RM(bp)) {
>     bp->hw_resc.resv_tx_rings = hwr->tx;  // Partial state update only
>     return 0;
> }
> ```
> 
> This should also be addressed to maintain state consistency.
> 
> ## References
> 
> - **Related CVE**: 
> CVE-2024-44933 (bnxt RSS out-of-bounds)
> - **Linux Kernel Source**: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> - **Broadcom bnxt Driver**: 
> `drivers/net/ethernet/broadcom/bnxt/`
> - **Original Fix**: 
> https://lore.kernel.org/netdev/

This link doesn't provide any value. Its just pointing to the lore list
archive...

The CVE linked *is* a related fix, but actually has valid data and a
crash dump backing it up:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=abd573e9ad2ba64eaa6418a5f4eec819de28f205

https://lore.kernel.org/netdev/20240806053742.140304-1-michael.chan@broadcom.com/

> 
> Signed-off-by: qianjiaru <qianjiaru77@...il.com>
> ---
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 207a8bb36..b59ce7f45 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -13610,8 +13610,11 @@ bool bnxt_rfs_capable(struct bnxt *bp, bool new_rss_ctx)
>  		return false;
>  	}
>  
> -	if (!BNXT_NEW_RM(bp))
> -		return true;
> +    // FIXED: Apply consistent validation for all firmware versions
> +    if (!BNXT_NEW_RM(bp)) {
> +        // Basic validation even for old firmware
> +        return (hwr.vnic <= max_vnics && hwr.rss_ctx <= max_rss_ctxs);

Literally 4 lines above this,  just outside the context, we already
check these exact things:

>         if (hwr.vnic > max_vnics || hwr.rss_ctx > max_rss_ctxs) {
>                 if (bp->rx_nr_rings > 1)
>                         netdev_warn(bp->dev,
>                                     "Not enough resources to support NTUPLE filters, enough resources for up to %d rx rings\n",
>                                     min(max_rss_ctxs - 1, max_vnics - 1));
>                 return false;
>         }

To me, this smells like an AI-generated "fix" which lacks substance. It
appears as if some scanning tool found similar looking code and flagged
it, without understanding the context and realizing that the validation
is already taken care of.

Whether this change was actually generated or not, it clearly doesn't
fix any real issue or provide value. Please don't waste reviewers time.


Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)

Powered by blists - more mailing lists