Re: [PATCH net-next v5 11/14] net: renesas: rswitch: clean up is_rdev rswitch_device checking

[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.
---
net: renesas: rswitch: clean up is_rdev rswitch_device checking

This commit cleans up the rswitch_device checking by replacing the
rswitch_port_check wrapper with direct calls to is_rdev. It also adds
a null pointer check for the net_device pointer in is_rdev.

> diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c b/drivers/net/ethernet/renesas/rswitch_l2.c
> index 3cc0ce8762f3c..c76fc8121ef05 100644
> --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> +++ b/drivers/net/ethernet/renesas/rswitch_l2.c

[ ... ]

> @@ -278,13 +273,13 @@ static int rswitch_switchdev_blocking_event(struct notifier_block *nb,
>  		return -EOPNOTSUPP;

[Severity: High]
This is a pre-existing issue, but does this unconditionally intercept
and reject port object addition and deletion events for all devices
in the system?

Returning -EOPNOTSUPP directly without checking if the device belongs to
the rswitch driver stops the global notification chain prematurely
because raw negative values have the NOTIFY_STOP_MASK bit set. Could
this break switchdev object offloading (such as VLAN configurations)
for all other unrelated network interfaces in the system?

I note this is fixed later in the series by commit 17583b9d88b1b
("net: renesas: rswitch: add vlan aware switching").

>  	case SWITCHDEV_PORT_ATTR_SET:
>  		ret = switchdev_handle_port_attr_set(ndev, ptr,
> -						     rswitch_port_check,
> +						     is_rdev,
>  						     rswitch_port_attr_set);
>  		break;
>  	default:
> -		if (!rswitch_port_check(ndev))
> -			return NOTIFY_DONE;
> -		ret = -EOPNOTSUPP;
> +		if (is_rdev(ndev))
> +			return -EOPNOTSUPP;

[Severity: High]
Does returning a raw negative error code directly from the notifier
callback cause the kernel to misinterpret the failure as a success
and abruptly halt the notifier chain?

By returning -EOPNOTSUPP directly instead of assigning it to ret and
relying on notifier_from_errno() at the end of the function, this
returns a negative integer. The binary representation has the
NOTIFY_STOP_MASK bit set, which incorrectly halts the entire notifier
chain.

Furthermore, when the switchdev core decodes this value using
notifier_to_errno(), the stripped integer evaluates to 0 (success).
Could this cause the kernel to silently assume unsupported operations
on rswitch devices succeeded?

I see this was also fixed later in the series in commit 17583b9d88b1b
("net: renesas: rswitch: add vlan aware switching").

> +		return NOTIFY_DONE;
>  	}
>  
>  	return notifier_from_errno(ret);