linux-kernel - Re: [PATCH v2 09/10] gpio: aggregator: cancel deferred probe for devices created via configfs

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: <CAMRc=Meb633zVgemPSeNtnm8oJmk=njcr2CQQbD5UJd=tBC5Zg@mail.gmail.com>
Date: Tue, 4 Feb 2025 14:14:27 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Koichiro Den <koichiro.den@...onical.com>, geert+renesas@...der.be
Cc: linux-gpio@...r.kernel.org, linus.walleij@...aro.org, 
	maciej.borzecki@...onical.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 09/10] gpio: aggregator: cancel deferred probe for
 devices created via configfs

On Mon, Feb 3, 2025 at 4:12 AM Koichiro Den <koichiro.den@...onical.com> wrote:
>
> For aggregators initialized via configfs, write 1 to 'live' waits for
> probe completion and returns an error if the probe fails, unlike the
> legacy sysfs interface, which is asynchronous.
>
> Since users control the liveness of the aggregator device and might be
> editting configurations while 'live' is 0, deferred probing is both
> unnatural and unsafe.
>
> Cancel deferred probe for purely configfs-based aggregators when probe
> fails.
>
> Signed-off-by: Koichiro Den <koichiro.den@...onical.com>
> ---
>  drivers/gpio/gpio-aggregator.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> index e101b78ad524..174078e02287 100644
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -1313,7 +1313,6 @@ static struct attribute *gpio_aggregator_attrs[] = {
>  };
>  ATTRIBUTE_GROUPS(gpio_aggregator);
>
> -
>  /*
>   *  GPIO Aggregator platform device
>   */
> @@ -1342,8 +1341,22 @@ static int gpio_aggregator_probe(struct platform_device *pdev)
>
>         for (i = 0; i < n; i++) {
>                 descs[i] = devm_gpiod_get_index(dev, NULL, i, GPIOD_ASIS);
> -               if (IS_ERR(descs[i]))
> +               if (IS_ERR(descs[i])) {
> +                       /*
> +                        * Deferred probing is not suitable when the aggregator
> +                        * is created by userspace. They should just retry later
> +                        * whenever they like. For device creation via sysfs,
> +                        * error is propagated without overriding for backward
> +                        * compatibility. .prevent_deferred_probe is kept unset
> +                        * for other cases.
> +                        */
> +                       if (!init_via_sysfs && !dev_of_node(dev) &&
> +                           descs[i] == ERR_PTR(-EPROBE_DEFER)) {
> +                               pr_warn("Deferred probe canceled for creation by userspace.\n");
> +                               return -ENODEV;
> +                       }
>                         return PTR_ERR(descs[i]);
> +               }
>         }
>
>         features = (uintptr_t)device_get_match_data(dev);
> --
> 2.45.2
>

Geert, what do you think about making the sysfs interface synchronous
instead? I would argue it's actually more logical as the user will
instinctively expect the aggregator to be ready after write() to
new_device returns.

Bart