All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: wei.fang@oss.nxp.com
Cc: Frank.Li@kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4
Date: Wed, 29 Jul 2026 02:29:15 +0000	[thread overview]
Message-ID: <20260729022916.490F91F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260728023217.2499420-3-wei.fang@oss.nxp.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] State desynchronization due to TOCTOU race condition updating the newly added software flag.

Pre-existing issues:
- [Critical] Unprotected read-modify-write of hardware registers leads to DMA memory corruption (Use-After-Free).
--

--- Patch [2]: [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 ---
commit c984fc5e7d3c90c2dfd0f53ae9ad32adf715c689
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: restore RX ring congestion mode for ENETC v4

This commit fixes an issue where the RX BD ring congestion mode is cleared
when RX BD rings are rebuilt at runtime without a link status change. It
tracks the desired CM state in the software flag ENETC_RXBDR_CM so it can
be restored during reconfiguration.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 437a15bbb47ba..8db5a47fdbae4 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -748,6 +748,7 @@ static void enetc4_set_rx_pause(struct enetc_pf *pf, bool rx_pause)
>  
>  static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_pause)
>  {
> +	struct enetc_ndev_priv *priv = netdev_priv(pf->si->ndev);
>  	u32 pause_off_thresh = 0, pause_on_thresh = 0;
>  	u32 init_quanta = 0, refresh_quanta = 0;
>  	struct enetc_hw *hw = &pf->si->hw;
> @@ -764,6 +765,8 @@ static void enetc4_set_tx_pause(struct enetc_pf *pf, int num_rxbdr, bool tx_paus
>  	int i;
>  
>  	for (i = 0; i < num_rxbdr; i++) {
>  		old_rbmr = enetc_rxbdr_rd(hw, i, ENETC_RBMR);
>  		rbmr = u32_replace_bits(old_rbmr, tx_pause ? 1 : 0, ENETC_RBMR_CM);
>  		if (rbmr == old_rbmr)
>  			continue;
>  
>  		enetc_rxbdr_wr(hw, i, ENETC_RBMR, rbmr);
>  	}

[Severity: Critical]
This is a pre-existing issue, but could this unprotected read-modify-write
lead to memory corruption if it races with enetc_disable_rxbdr()?

If phylink_resolve() executes enetc4_set_tx_pause() concurrently with
enetc_reconfigure(), this loop can read ENETC_RBMR before the ring is
torn down. 

If it writes the modified value back after enetc_disable_rxbdr()
clears the enable bit, it would inadvertently re-enable the RX ring in
hardware:

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_disable_rxbdr() {
    ...
    /* disable EN bit on ring */
    enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
    ...
}

Because the software frees the associated DMA buffers during teardown, the
hardware would then DMA incoming packets directly into freed memory.

>  
>  	if (tx_pause) {
> +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> +

[Severity: High]
Can this result in the software state and hardware register becoming
desynchronized?

Since ENETC_RXBDR_CM is set after the hardware registers are updated in the
loop above, there appears to be a race window. If enetc_setup_rxbdr() is
called by a concurrent enetc_reconfigure() during this window, it would
see the unset flag:

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_setup_rxbdr() {
    ...
    if (test_bit(ENETC_RXBDR_CM, &priv->flags))
        rbmr |= ENETC_RBMR_CM;
    ...
}

This would overwrite the hardware register without the CM bit. Then this
thread sets the software flag, leaving flow control disabled in hardware
when the software state expects it to be enabled.

>  		/* When the port first enters congestion, send a PAUSE request
>  		 * with the maximum number of quanta. When the port exits

[ ... ]

> @@ -949,7 +954,10 @@ static void enetc4_pl_mac_link_down(struct phylink_config *config,
>  				    phy_interface_t interface)
>  {
>  	struct enetc_pf *pf = phylink_to_enetc_pf(config);
> +	struct enetc_ndev_priv *priv;
>  
> +	priv = netdev_priv(pf->si->ndev);
> +	clear_bit(ENETC_RXBDR_CM, &priv->flags);
>  	enetc4_mac_rx_graceful_stop(pf);
>  	enetc4_mac_tx_graceful_stop(pf);
>  }

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

  reply	other threads:[~2026-07-29  2:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28  2:32 [PATCH net 0/2] net: enetc: restore RX ring congestion mode after ring reconfiguration wei.fang
2026-07-28  2:32 ` [PATCH net 1/2] " wei.fang
2026-07-29  2:29   ` sashiko-bot
2026-07-29  7:00     ` Wei Fang (OSS)
2026-07-28  2:32 ` [PATCH net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 wei.fang
2026-07-29  2:29   ` sashiko-bot [this message]
2026-07-29  6:16     ` Wei Fang (OSS)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260729022916.490F91F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=wei.fang@oss.nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.