linux-kernel - Re: [PATCH v2 1/3] gpio: introduce utilities for synchronous fake device creation

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=MdhkiaDs8t9BYveYhy86+svQkHnPxhGx56AMOs=7n9mcQ@mail.gmail.com>
Date: Thu, 20 Feb 2025 14:13:42 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Koichiro Den <koichiro.den@...onical.com>
Cc: linux-gpio@...r.kernel.org, geert+renesas@...der.be, 
	linus.walleij@...aro.org, maciej.borzecki@...onical.com, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] gpio: introduce utilities for synchronous fake
 device creation

On Thu, Feb 20, 2025 at 2:07 PM Koichiro Den <koichiro.den@...onical.com> wrote:
>
> On Thu, Feb 20, 2025 at 12:06:33PM GMT, Bartosz Golaszewski wrote:
> > On Tue, Feb 18, 2025 at 5:04 PM Koichiro Den <koichiro.den@...onical.com> wrote:
> > >
> > > Both gpio-sim and gpio-virtuser share a mechanism to instantiate a
> > > platform device, wait for probe completion, and retrieve the probe
> > > success or error status synchronously. With gpio-aggregator planned to
> > > adopt this approach for its configfs interface, it's time to factor
> > > out the common code.

[snip]

> > > +void dev_sync_probe_init(struct dev_sync_probe_data *data)
> > > +{
> > > +       memset(data, 0, sizeof(*data));
> > > +       init_completion(&data->probe_completion);
> > > +       data->bus_notifier.notifier_call = dev_sync_probe_notifier_call;
> > > +}
> > > +EXPORT_SYMBOL_GPL(dev_sync_probe_init);
> > > +
> > > +int dev_sync_probe_register(struct dev_sync_probe_data *data,
> > > +                           struct platform_device_info *pdevinfo)
> > > +{
> > > +       struct platform_device *pdev;
> > > +       char *name;
> > > +
> > > +       name = kasprintf(GFP_KERNEL, "%s.%u", pdevinfo->name, pdevinfo->id);
> >
> > pdevinfo->id is a signed integer
> >
> > I'm also wondering if we could avoid the allocation here and keep on
> > using snprintf() like in the existing drivers? On the other hand,
> > memory is cheap so no big deal.
>
> Are you assuming the following change?
>
>    struct dev_sync_probe_data {
>           struct platform_device *pdev;
>   -       const char *name;
>   +       char name[32];
>
>           /* Synchronize with probe */
>           struct notifier_block bus_notifier;
>

No, I was thinking about a local buffer in the notifier handler, like
what we do currently in gpio-sim, but no worries, you can keep it this
way.

> >
> > > +       if (!name)
> > > +               return -ENOMEM;
> > > +
> > > +       data->driver_bound = false;
> > > +       data->name = name;
> > > +       reinit_completion(&data->probe_completion);
> > > +       bus_register_notifier(&platform_bus_type, &data->bus_notifier);
> > > +
> > > +       pdev = platform_device_register_full(pdevinfo);
> > > +       if (IS_ERR(pdev)) {
> > > +               bus_unregister_notifier(&platform_bus_type, &data->bus_notifier);
> > > +               kfree(data->name);
> >
> > We could probably simplify it by using __free(kfree) with the name
> > variable and just setting it at the end with no_free_ptr().
>
> platform_device_register_full() call path might finish probe so before
> calling it, we need to make sure the 'name' is filled in. That's why I
> didn't used __free(kfree).
>

Not sure I understand this. Would you mind rephrasing?

Bart