Re: [PATCH net-next v10 3/4] net: dsa: motorcomm: Add LED support

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Unconditional kernel panic during probe due to pointer arithmetic on NULL net_device pointers in yt921x_cled_hw_control_get_device().
--

--- Patch [3]: [PATCH net-next v10 3/4] net: dsa: motorcomm: Add LED support ---
commit 25a5fb03fc03b52bd3ea80525b1fd57bd6de812d
Author: David Yang <[email protected]>

net: dsa: motorcomm: Add LED support

This commit adds support for controlling the LEDs attached to the
Motorcomm YT9215 switch chips via the standard LEDs subsystem. It
currently supports parallel mode and strict 1:1 mapping.

> diff --git a/drivers/net/dsa/motorcomm/leds.c b/drivers/net/dsa/motorcomm/leds.c
> new file mode 100644
> index 0000000000000..e7ef0ef80a266
> --- /dev/null
> +++ b/drivers/net/dsa/motorcomm/leds.c

[ ... ]

> +static struct device * __maybe_unused
> +yt921x_cled_hw_control_get_device(struct led_classdev *led_cdev)
> +{
> +	struct yt921x_led *led = to_yt921x_led(led_cdev);
> +	struct yt921x_port *pp = to_yt921x_port(led);
> +	struct yt921x_priv *priv = to_yt921x_priv(pp);
> +	struct dsa_port *dp;
> +
> +	dp = dsa_to_port(&priv->ds, pp->index);
> +	if (!dp)
> +		return NULL;
> +
> +	if (dsa_port_is_user(dp))
> +		return &dp->user->dev;
> +	if (dsa_port_is_cpu(dp))
> +		return &dp->conduit->dev;

[Severity: Critical]
Could this cause a kernel panic during probe if dp->user or dp->conduit is
NULL?

When the hw_control_trigger is set to netdev during driver setup, the LED
core immediately activates the netdev trigger, which in turn calls
yt921x_cled_hw_control_get_device().

At this point during probe, the user ports are not fully instantiated yet,
meaning dp->user is NULL. Unconditionally returning &dp->user->dev performs
pointer arithmetic on the NULL pointer, yielding a non-zero invalid pointer
because dev is located at an offset within struct net_device.

The netdev trigger core evaluates this pointer as true and calls to_net_dev()
on it, which subtracts the offset and yields exactly NULL. It then attempts
to dereference the network device name, crashing the kernel.

Does this need an explicit check to verify dp->user and dp->conduit are valid
pointers before accessing their dev members?

> +
> +	return NULL;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3