Re: [PATCH v4 5/5] phy: phy-can-transceiver: Drop unused include

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The probe function incorrectly aborts when match data is found and dereferences a NULL pointer when match data is absent.
--

commit 8e35724d73ac65aed30d62ac9342ff1ac6be9401
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

phy: phy-can-transceiver: Drop unused include

This commit removes the unused <linux/gpio.h> legacy header from the PHY CAN
transceiver driver, as its symbols are not used in the file. It also adjusts
the header order slightly.

> diff --git a/drivers/phy/phy-can-transceiver.c b/drivers/phy/phy-can-transceiver.c
> index 973cc7ce651fb..534a4fc482aa8 100644
> --- a/drivers/phy/phy-can-transceiver.c
> +++ b/drivers/phy/phy-can-transceiver.c
> @@ -5,12 +5,11 @@
>   * Copyright (C) 2021 Texas Instruments Incorporated - https://www.ti.com
>   *
>   */
> +#include <linux/gpio/consumer.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
>  #include <linux/property.h>
>  #include <linux/module.h>
> -#include <linux/gpio.h>
> -#include <linux/gpio/consumer.h>
>  #include <linux/mux/consumer.h>
>  
>  struct can_transceiver_data {

While reviewing this patch, I noticed an issue introduced earlier in the
series by commit "phy: phy-can-transceiver: use device_get_match_data()":

> --- a/drivers/phy/phy-can-transceiver.c
> +++ b/drivers/phy/phy-can-transceiver.c
[ ... ]
> @@ -161,11 +160,10 @@ static int can_transceiver_phy_probe(struct platform_device *pdev)
>  	u32 max_bitrate = 0;
>  	int err, i, num_ch = 1;
>  
> -	match = of_match_node(can_transceiver_phy_ids, pdev->dev.of_node);
> -	if (!match || !match->data)
> +	drvdata = device_get_match_data(dev);
> +	if (drvdata)
>  		return -ENODEV;
>  
> -	drvdata = match->data;
>  	if (drvdata->flags & CAN_TRANSCEIVER_DUAL_CH)
>  		num_ch = 2;

Does this incorrectly abort the probe when match data is successfully found?
It looks like the condition is inverted, returning -ENODEV upon success.

Additionally, if match data is absent, drvdata will be NULL and execution
continues to the next line where it is dereferenced. Can this cause a NULL
pointer dereference?

Should the check be corrected to if (!drvdata)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512130552.272476-1-andriy.shevchenko@linux.intel.com?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy