Re: [PATCH net-next 2/3] net: rmnet: annotate data-races around mux_id

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] The patch "net: rmnet: annotate data-races around mux_id"
  contains, besides the READ_ONCE()/WRITE_ONCE() annotations, a real…

--

net: rmnet: annotate data-races around mux_id

This adds READ_ONCE()/WRITE_ONCE() annotations around priv->mux_id and
ep->mux_id, which are written under RTNL but read from the transmit and
receive paths.  It also reorders rmnet_changelink() so that ep->mux_id is
published before the endpoint is inserted into its new hash bucket.

> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> index 59ef8b4ce5321..248a9d822409b 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
> @@ -331,11 +331,11 @@ static int rmnet_changelink(struct net_device *dev, struct nlattr *tb[],
>  			}
>  
>  			hlist_del_init_rcu(&ep->hlnode);
> +			WRITE_ONCE(ep->mux_id, mux_id);
>  			hlist_add_head_rcu(&ep->hlnode,
>  					   &port->muxed_ep[mux_id]);
>  
> -			ep->mux_id = mux_id;
> -			priv->mux_id = mux_id;
> +			WRITE_ONCE(priv->mux_id, mux_id);

[Severity: Low]
Beyond the annotations, the reordering here looks like a functional fix
rather than a pure cleanup.  Should it carry a Fixes: tag?

With the old ordering the endpoint was visible in the new bucket while
still carrying its old mux_id, so a concurrent reader in
rmnet_get_endpoint() would reject it:

	hlist_for_each_entry_rcu(ep, &port->muxed_ep[mux_id], hlnode,
				 lockdep_rtnl_is_held()) {
		if (READ_ONCE(ep->mux_id) == mux_id)
			return ep;
	}

	return NULL;

and the RX path then frees the skb in __rmnet_map_ingress_handler():

	ep = rmnet_get_endpoint(port, mux_id);
	if (!ep)
		goto free_skb;

The reader is driven by inbound traffic in
rmnet_rx_handler() -> rmnet_map_ingress_handler() ->
__rmnet_map_ingress_handler(), while the writer side runs from
ip link set <rmnet dev> type rmnet mux_id N, so the observable effect is
occasional packet loss during a mux_id change.

git blame points at commit 2abb5792387e ("net: qualcomm: rmnet: Allow
configuration updates to existing devices") for the previous ordering, so
would the following trailer be appropriate to help stable pick this up?

	Fixes: 2abb5792387e ("net: qualcomm: rmnet: Allow configuration updates to existing devices")

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260916132231.1780091-1-edumazet%40google.com