RE: [PATCH net] net: libwx: fix concurrent bitmap overwrite in PTP setup

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Mon, Aug 17, 2026 9:07 PM, Vadim Fedorenko wrote:
> On 17/08/2026 03:17, Jiawen Wu wrote:
> > In wx_ptp_set_timestamp_mode(), the driver copies the global `wx->flags`
> > bitmap to a local variable, modifies the PTP-related bits, and then writes
> > the entire bitmap back using memcpy().
> >
> > This Read-Copy-Update pattern is unsafe and introduces a critical race
> > condition. Other asynchronous contexts (such as Tx timeout routines or
> > GPIO IRQ handlers) update individual bits in `wx->flags` concurrently
> > using atomic bitops like set_bit() or clear_bit(). The memcpy() write-back
> > can silently overwrite and drop these concurrent changes, potentially
> > causing the driver to miss critical module reset or PCIe recovery requests.
> >
> > Fix this by removing the local bitmap copy. Instead, evaluate the intended
> > PTP flag states locally and apply them directly to `wx->flags` using
> > atomic set_bit() and clear_bit() operations only after the hardware is
> > successfully configured.
> >
> > Fixes: 06e75161b9d4 ("net: wangxun: Add support for PTP clock")
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > ---
> >   drivers/net/ethernet/wangxun/libwx/wx_ptp.c | 22 ++++++++++-----------
> >   1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> > index 44f3e6505246..7d4bd7b258b7 100644
> > --- a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> > +++ b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> > @@ -555,13 +555,12 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
> >   {
> >   	u32 tsync_tx_ctl = WX_TSC_1588_CTL_ENABLED;
> >   	u32 tsync_rx_ctl = WX_PSR_1588_CTL_ENABLED;
> > -	DECLARE_BITMAP(flags, WX_PF_FLAGS_NBITS);
> >   	u32 tsync_rx_mtrl = PTP_EV_PORT << 16;
> > +	bool rx_in_register = false;
> > +	bool rx_enabled = false;
> 
> why do you need 2 variables if their values are equal in all cases?

You are right!

> 
> >   	bool is_l2 = false;
> >   	u32 regval;
> >
> > -	memcpy(flags, wx->flags, sizeof(wx->flags));
> > -
> >   	switch (config->tx_type) {
> >   	case HWTSTAMP_TX_OFF:
> >   		tsync_tx_ctl = 0;
> > @@ -576,20 +575,18 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
> >   	case HWTSTAMP_FILTER_NONE:
> >   		tsync_rx_ctl = 0;
> >   		tsync_rx_mtrl = 0;
> > -		clear_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, flags);
> > -		clear_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, flags);
> >   		break;
> >   	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> >   		tsync_rx_ctl |= WX_PSR_1588_CTL_TYPE_L4_V1;
> >   		tsync_rx_mtrl |= WX_PSR_1588_MSG_V1_SYNC;
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, flags);
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, flags);
> > +		rx_enabled = true;
> > +		rx_in_register = true;
> >   		break;
> >   	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> >   		tsync_rx_ctl |= WX_PSR_1588_CTL_TYPE_L4_V1;
> >   		tsync_rx_mtrl |= WX_PSR_1588_MSG_V1_DELAY_REQ;
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, flags);
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, flags);
> > +		rx_enabled = true;
> > +		rx_in_register = true;
> >   		break;
> >   	case HWTSTAMP_FILTER_PTP_V2_EVENT:
> >   	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> > @@ -602,9 +599,9 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
> >   	case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> >   		tsync_rx_ctl |= WX_PSR_1588_CTL_TYPE_EVENT_V2;
> >   		is_l2 = true;
> > +		rx_enabled = true;
> > +		rx_in_register = true;
> >   		config->rx_filter = HWTSTAMP_FILTER_PTP_V2_EVENT;
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, flags);
> > -		set_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, flags);
> >   		break;
> >   	default:
> >   		/* register PSR_1588_MSG must be set in order to do V1 packets,
> > @@ -643,7 +640,8 @@ static int wx_ptp_set_timestamp_mode(struct wx *wx,
> >   	WX_WRITE_FLUSH(wx);
> >
> >   	/* configure adapter flags only when HW is actually configured */
> > -	memcpy(wx->flags, flags, sizeof(wx->flags));
> > +	assign_bit(WX_FLAG_RX_HWTSTAMP_ENABLED, wx->flags, rx_enabled);
> > +	assign_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, wx->flags, rx_in_register);
> >
> >   	/* clear TX/RX timestamp state, just to be sure */
> >   	wx_ptp_clear_tx_timestamp(wx);
> 
>