linux-kernel - Re: [PATCH v2 05/11] gpiolib: provide gpio_device_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZQBEtxYO1hb4KJ1V@smile.fi.intel.com>
Date:   Tue, 12 Sep 2023 14:00:07 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Bartosz Golaszewski <brgl@...ev.pl>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-acpi@...r.kernel.org,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 05/11] gpiolib: provide gpio_device_get_desc()

On Tue, Sep 12, 2023 at 12:07:21PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> 
> Getting the GPIO descriptor directly from the gpio_chip struct is
> dangerous as we don't take the reference to the underlying GPIO device.
> In order to start working towards removing gpiochip_get_desc(), let's
> provide a safer variant that works with an existing reference to struct
> gpio_device.

...

> +struct gpio_desc *
> +gpio_device_get_desc(struct gpio_device *gdev, unsigned int hwnum)
> +{
> +	struct gpio_chip *gc = gdev->chip;

I prefer

	struct gpio_chip *gc;

> +	/*
> +	 * FIXME: This will be locked once we protect gdev->chip everywhere
> +	 * with SRCU.
> +	 */

	gc = gdev->chip;

as it is more robust against changes in between and easier to read and
understand in the code what's going on. With decoupled assignment in this case
it's harder to see at the flash glance if the gc is parameter of the function
or being derived from somewhere else.

> +	if (!gc)
> +		return ERR_PTR(-ENODEV);
>  
>  	if (hwnum >= gdev->ngpio)
>  		return ERR_PTR(-EINVAL);
>  
>  	return &gdev->descs[hwnum];
>  }

-- 
With Best Regards,
Andy Shevchenko