Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unprotected read-modify-write of the ENETC4_PM_CMD_CFG(0) register can cause configuration loss via a TOCTOU race.
--
--- Patch [2]: [PATCH v2 net 2/2] net: enetc: restore RX ring congestion mode for ENETC v4 ---
commit bd5bd2aaf8ee0b0d3b280c89c74f1cacc1556562
Author: Wei Fang <[email protected]>
net: enetc: restore RX ring congestion mode for ENETC v4
This commit fixes an issue where the RX BD ring congestion mode is cleared
during runtime reconfiguration and never restored. It tracks the desired
state in a software flag and restores it when rings are re-enabled,
ensuring flow control continues to work.
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index fcfbabb29d22e..9bb1004548abc 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -718,22 +718,14 @@ static void enetc4_set_rx_pause(struct enetc_pf *pf, bool rx_pause)
> enetc_port_mac_wr(si, ENETC4_PM_CMD_CFG(0), val);
> }
[Severity: High]
This is a pre-existing issue, but does this read-modify-write on
ENETC4_PM_CMD_CFG(0) need to be protected by si->gen_lock?
It appears enetc4_set_rx_pause() and enetc4_mac_tx_enable() run from the
phylink_resolve workqueue, which does not hold rtnl_lock. Meanwhile,
ethtool commands (like changing loopback) trigger enetc4_pf_set_loopback()
under rtnl_lock.
Since both paths perform lockless read-modify-write operations on the
same ENETC4_PM_CMD_CFG(0) register, could a concurrent link state change
and ethtool command result in a race condition where one of the updates
is overwritten and lost?
--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.