Re: [PATCH net-next v3 12/12] dpaa2-switch: add support for imprecise source port

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
dpaa2-switch: add support for imprecise source port

Switch ports configured as part of a LAG group are not able to provide
a precise source port for all packets which reach the control interface.

The only frames which will have a precise source port are those that are
explicitly trapped, for example STP and LCAP frames. For any other
frames (for example, those which are flooded) we can only know the
ingress LAG group.

Take into account the DPAA2_ETHSW_FLC_IMPRECISE_IF_ID bit and based on
its value target the bond device or the specific source netdevice.

> diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> index 7aef34778e220..163f6c51c1606 100644
> --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> @@ -3270,8 +3272,10 @@ static void dpaa2_switch_rx(struct dpaa2_switch_fq *fq,
>  	rcu_read_lock();
>  
>  	lag = rcu_dereference(port_priv->lag);
> -
> -	skb->dev = netdev;
> +	if (DPAA2_ETHSW_FLC_IMPRECISE_IF_ID(flc) && lag)
> +		skb->dev = lag->bond_dev;

[Severity: High]
By assigning skb->dev = lag->bond_dev here, isn't there a risk of a
use-after-free on PREEMPT_RT?

The explicit rcu_read_lock() is dropped before handing the SKB over to
netif_receive_skb(). While NAPI's local_bh_disable() provides RCU protection
on standard kernels, softirqs are threaded on PREEMPT_RT and do not provide
an implicit RCU read-side critical section.

Between the rcu_read_unlock() and the internal rcu_read_lock() taken by
netif_receive_skb_core(), the threaded softirq can be preempted. If the bond
device is concurrently unregistered, synchronize_net() will not wait for the
NAPI thread, allowing bond_dev to be freed before netif_receive_skb()
dereferences it.

> +	else
> +		skb->dev = netdev;
>  	skb->protocol = eth_type_trans(skb, skb->dev);
>  
>  	/* Setup the offload_fwd_mark only if the port is under a bridge