RE: [PATCH net 1/2] net: enetc: restore RX ring congestion mode after ring reconfiguration

> > --- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> > @@ -633,6 +633,8 @@ static void enetc_pl_mac_link_up(struct
> phylink_config *config,
> >  	}
> >
> >  	if (tx_pause) {
> > +		set_bit(ENETC_RXBDR_CM, &priv->flags);
> > +
> 
> [Severity: Critical]
> This is a pre-existing issue, but can this lockless read-modify-write on
> the ENETC_RBMR register race with driver reconfiguration?
> 
> Looking at enetc_pl_mac_link_up() in enetc_pf.c, there is an unsynchronized
> read-modify-write loop on ENETC_RBMR just before this flag update:
> 
> 	for (idx = 0; idx < priv->num_rx_rings; idx++) {
> 		rbmr = enetc_rxbdr_rd(hw, idx, ENETC_RBMR);
> 		...
> 		enetc_rxbdr_wr(hw, idx, ENETC_RBMR, rbmr);
> 	}
> 
> If phylink's mac_link_up executes asynchronously in the phylink_resolve
> workqueue, could it run concurrently with a user triggering a driver
> reconfiguration under rtnl_lock (like attaching an XDP program)?
> 
> If so, enetc_reconfigure() might call enetc_disable_rxbdr(), which clears
> the ENETC_RBMR register to disable the ring:
> 
> 	/* disable EN bit on ring */
> 	enetc_rxbdr_wr(hw, idx, ENETC_RBMR, 0);
> 
> If mac_link_up() reads the ENETC_RBMR register before it is cleared by
> reconfiguration, and writes it back after it is cleared, it will write
> back the stale ENETC_RBMR_EN (enable) bit.
> 
> Since enetc_reconfigure() proceeds to call enetc_free_rxtx_rings() to free
> the DMA memory, re-enabling the hardware RX ring at this moment would
> cause
> the hardware to DMA incoming network packets into freed kernel memory.
> 
> Additionally, because the new ENETC_RXBDR_CM software flag is set here after
> the hardware read-modify-write loop, concurrent readers like
> enetc_setup_rxbdr() could observe a stale flag state and permanently lose
> flow control.
> 
> >  		/* When the port first enters congestion, send a PAUSE request
> >  		 * with the maximum number of quanta. When the port exits
> [ ... ]

I will improve this patch set to solve the pre-existing issue.

--
pw-bot: cr