Re: [net-next,v3,13/13] net: renesas: rswitch: add vlan aware switching

[Paolo Abeni]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net: renesas: rswitch: add vlan aware switching

This commit adds vlan support to the L2 HW bridge, with limited support
on R-Car S4 (not fully 802.1Q compliant).

> diff --git a/drivers/net/ethernet/renesas/Kconfig b/drivers/net/ethernet/renesas/Kconfig
> index 9b7559c88bee..2494ff60e8c2 100644
> --- a/drivers/net/ethernet/renesas/Kconfig
> +++ b/drivers/net/ethernet/renesas/Kconfig

[ ... ]

> diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
> index ea95a87ed234..b7d0e7f94ecd 100644
> --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> +++ b/drivers/net/ethernet/renesas/rswitch_l2.c

[ ... ]

> @@ -212,61 +197,397 @@ static int rswitch_netdevice_event(struct notifier_block *nb,

[ ... ]

> +static int rswitch_handle_port_obj_add(struct net_device *ndev,
> +				       struct notifier_block *nb,
> +				       struct switchdev_notifier_port_obj_info *info)
> +{
> +	struct switchdev_obj_port_vlan *p_vlan = SWITCHDEV_OBJ_PORT_VLAN(info->obj);
> +	struct rswitch_private *priv;
> +	int err;
> +
> +	priv = container_of(nb, struct rswitch_private, rswitch_switchdev_blocking_nb);
> +
> +	if ((p_vlan->flags & BRIDGE_VLAN_INFO_MASTER) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_END) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_ONLY_OPTS))
> +		return NOTIFY_DONE;
> +
> +	switch (info->obj->id) {
> +	case SWITCHDEV_OBJ_ID_PORT_VLAN:

Can the flag checks access the wrong memory? The code casts info->obj to
switchdev_obj_port_vlan before checking info->obj->id. When the notifier
delivers a non-VLAN object (such as SWITCHDEV_OBJ_ID_PORT_MDB for multicast
group changes), SWITCHDEV_OBJ_PORT_VLAN() performs an incorrect container_of
cast, and p_vlan->flags reads bytes from a different struct type.

For switchdev_obj_port_mdb, the p_vlan->flags field overlaps with the first
2 bytes of the addr[ETH_ALEN] MAC address field. Multicast MAC addresses
always have bit 0 of the first byte set, which matches BRIDGE_VLAN_INFO_MASTER
(1<<0), causing the function to return NOTIFY_DONE instead of falling through
to the default case that returns -EOPNOTSUPP.

Other switchdev drivers (lan966x, mlxsw, cpsw) perform the
SWITCHDEV_OBJ_PORT_VLAN cast inside the SWITCHDEV_OBJ_ID_PORT_VLAN case,
not before the type switch.

The same issue exists in rswitch_handle_port_obj_del().

> +		if (!is_rdev(ndev))
> +			err = rswitch_port_obj_do_add_gwca(ndev, priv, p_vlan);
> +		else
> +			err = rswitch_port_obj_do_add(ndev, p_vlan);
> +
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	info->handled = true;
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static int rswitch_handle_port_obj_del(struct net_device *ndev,
> +				       struct notifier_block *nb,
> +				       struct switchdev_notifier_port_obj_info *info)
> +{
> +	struct switchdev_obj_port_vlan *p_vlan = SWITCHDEV_OBJ_PORT_VLAN(info->obj);
> +	struct rswitch_private *priv;
> +	int err;
> +
> +	priv = container_of(nb, struct rswitch_private, rswitch_switchdev_blocking_nb);
> +
> +	if ((p_vlan->flags & BRIDGE_VLAN_INFO_MASTER) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_END) ||
> +	    (p_vlan->flags & BRIDGE_VLAN_INFO_ONLY_OPTS))
> +		return NOTIFY_DONE;
> +
> +	switch (info->obj->id) {
> +	case SWITCHDEV_OBJ_ID_PORT_VLAN:
> +		if (!is_rdev(ndev))
> +			err = rswitch_port_obj_do_del_gwca(ndev, priv, p_vlan);
> +		else
> +			err = rswitch_port_obj_do_del(ndev, p_vlan);
> +
> +		if (err < 0)
> +			return err;
> +
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	info->handled = true;
> +
> +	return NOTIFY_DONE;
> +}

[ ... ]