Re: [PATCH v9 15/36] pinctrl: airoha: add set_direction() helper for gpio_chip

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Delegating to `airoha_gpio_set_direction()` and `airoha_gpio_get_direction()` from pinmux/pinconf callbacks introduces a NULL pointer dereference during early pin controller initialization (e.g., processing device tree pin hogs).
--

--- Patch [15]: [PATCH v9 15/36] pinctrl: airoha: add set_direction() helper for gpio_chip ---
commit 284bb5b3beca75443ce82d7e07932f3712cf6811
Author: Mikhail Kshevetskiy <[email protected]>

pinctrl: airoha: add set_direction() helper for gpio_chip

The patch creates set_direction() helper for gpio_chip abstraction.
It also implements/reimplements some function using newly defined
helper.

> diff --git a/drivers/pinctrl/airoha/pinctrl-airoha.c b/drivers/pinctrl/airoha/pinctrl-airoha.c
> index df1694296046c..e4ccf357df892 100644
> --- a/drivers/pinctrl/airoha/pinctrl-airoha.c
> +++ b/drivers/pinctrl/airoha/pinctrl-airoha.c
> @@ -2539,12 +2539,42 @@ static int airoha_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio)
>  	return val & mask ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
>  }
>  
> +static int airoha_gpio_set_direction(struct gpio_chip *chip, unsigned int gpio,
> +				     bool input)
> +{
> +	struct airoha_pinctrl *pinctrl = gpiochip_get_data(chip);

[Severity: High]
Does this code introduce a NULL pointer dereference during early pin controller
initialization?

During airoha_pinctrl_probe(), pinctrl processing of device tree pin hogs can
occur before the GPIO chip is fully initialized. If a pin hog specifies
direction configurations, the pinctrl core will ultimately invoke
airoha_pinmux_set_direction().

That function now passes the uninitialized &pinctrl->gpiochip.chip into
airoha_gpio_set_direction(). When gpiochip_get_data(chip) is called here, it
dereferences chip->gpiodev, causing a panic since it is not initialized until
devm_gpiochip_add_data() runs later.

[ ... ]

> @@ -2761,27 +2791,13 @@ static int airoha_pinmux_set_direction(struct pinctrl_dev *pctrl_dev,
>  					unsigned int p, bool input)
>  {
>  	struct airoha_pinctrl *pinctrl = pinctrl_dev_get_drvdata(pctrl_dev);
> -	u32 mask, index;
> -	int err, pin;
> +	int pin;
>  
>  	pin = airoha_convert_pin_to_reg_offset(pctrl_dev, range, p);
>  	if (pin < 0)
>  		return pin;
>  
> -	/* set output enable */
> -	mask = BIT(pin % AIROHA_PIN_BANK_SIZE);
> -	index = pin / AIROHA_PIN_BANK_SIZE;
> -	err = regmap_update_bits(pinctrl->regmap, pinctrl->gpiochip.out[index],
> -				 mask, !input ? mask : 0);
> -	if (err)
> -		return err;
> -
> -	/* set direction */
> -	mask = BIT(2 * (pin % AIROHA_REG_GPIOCTRL_NUM_PIN));
> -	index = pin / AIROHA_REG_GPIOCTRL_NUM_PIN;
> -	return regmap_update_bits(pinctrl->regmap,
> -				  pinctrl->gpiochip.dir[index], mask,
> -				  !input ? mask : 0);
> +	return airoha_gpio_set_direction(&pinctrl->gpiochip.chip, pin, input);
>  }
>  
>  static const struct pinmux_ops airoha_pmxops = {

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