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

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 18/08/2026 08:47, 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>
> ---
> Changes in v2:
> - Combine two variables that are always equal into one.
> - Link to v1: https://lore.kernel.org/all/F97867A3AA2696FD+20260817021708.37975-1-jiawenwu@trustnetic.com
> ---
>   drivers/net/ethernet/wangxun/libwx/wx_ptp.c | 18 ++++++------------
>   1 file changed, 6 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..1165518d5522 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ptp.c
> @@ -555,13 +555,11 @@ 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_tstamp = false;
>   	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 +574,16 @@ 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_tstamp = 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_tstamp = true;
>   		break;
>   	case HWTSTAMP_FILTER_PTP_V2_EVENT:
>   	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> @@ -602,9 +596,8 @@ 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_tstamp = 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 +636,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_tstamp);
> +	assign_bit(WX_FLAG_RX_HWTSTAMP_IN_REGISTER, wx->flags, rx_tstamp);
>   
>   	/* clear TX/RX timestamp state, just to be sure */
>   	wx_ptp_clear_tx_timestamp(wx);

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>