Re: [PATCH] gpio: aggregator: fix a potential use-after-free

[Bartosz Golaszewski]

On Wed, May 20, 2026 at 11:15 AM Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Bartosz,
>
> On Wed, 20 May 2026 at 10:49, Bartosz Golaszewski
> <bartosz.golaszewski@xxxxxxxxxxxxxxxx> wrote:
> > On error we free aggr->lookups->dev_id before removing the entry from
> > the lookup table. If a concurrent thread calls gpiod_find() before we
> > remove the entry, it could iterate over the list and call
> > gpiod_match_lookup_table() which unconditionally dereferences dev_id
> > when calling strcmp(). Reverse the order of cleanup.
> >
> > Fixes: 86f162e73d2d ("gpio: aggregator: introduce basic configfs interface")
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/gpio/gpio-aggregator.c
> > +++ b/drivers/gpio/gpio-aggregator.c
> > @@ -979,8 +979,8 @@ static int gpio_aggregator_activate(struct gpio_aggregator *aggr)
> >  err_unregister_pdev:
> >         platform_device_unregister(pdev);
> >  err_remove_lookup_table:
> > -       kfree(aggr->lookups->dev_id);
> >         gpiod_remove_lookup_table(aggr->lookups);
> > +       kfree(aggr->lookups->dev_id);
> >  err_remove_swnode:
> >         fwnode_remove_software_node(swnode);
> >  err_remove_lookups:
>
> LGTM, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
>
> Note that gpio_aggregator_deactivate() does use the correct ordering.
> However, there is a second difference: gpio_aggregator_deactivate()
> does not have the call to fwnode_remove_software_node().
>
> I am not very familiar with swnodes. The kerneldoc for
> platform_device_info says:
>
>  * @swnode: a secondary software node to be attached to the device. The node
>  *      will be automatically registered and its lifetime tied to the platform
>  *      device if it is not registered yet.
>
> So perhaps the call to platform_device_unregister() takes care of
> that?  But then it should not be done again later, if we jumped to
> err_unregister_pdev, and not to a later label?
>

No, you have a good point. Other users of this pattern: gpio-sim and
gpio-virtuser do free the software node in their deactivate() path.

I'll send a separate fix.

Bartosz