Re: [PATCH v2 2/2] gpio: cdev: fix missed label sanitizing in debounce_setup()

From: Bartosz Golaszewski
Date: Thu Apr 04 2024 - 12:58:01 EST


On Thu, Apr 4, 2024 at 5:36 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxx> wrote:
>
> On Thu, Apr 04, 2024 at 11:33:28AM +0200, Bartosz Golaszewski wrote:
> > From: Kent Gibson <warthog618@xxxxxxxxx>
> >
> > When adding sanitization of the label, the path through
> > edge_detector_setup() that leads to debounce_setup() was overlooked.
> > A request taking this path does not allocate a new label and the
> > request label is freed twice when the request is released, resulting
> > in memory corruption.
> >
> > Add label sanitization to debounce_setup().
>
> ...
>
> > +static inline char *make_irq_label(const char *orig)
> > +{
> > + char *new;
> > +
> > + if (!orig)
> > + return NULL;
> > +
> > + new = kstrdup_and_replace(orig, '/', ':', GFP_KERNEL);
> > + if (!new)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + return new;
> > +}
> > +
> > +static inline void free_irq_label(const char *label)
> > +{
> > + kfree(label);
> > +}
>
> First of all this could have been done in the previous patch already, but okay.
>
> ...
>
> > + label = make_irq_label(line->req->label);
> > + if (IS_ERR(label))
> > + return -ENOMEM;
> > +
> > irqflags = IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING;
> > ret = request_irq(irq, debounce_irq_handler, irqflags,
> > line->req->label, line);
>
> But the main point how does this change fix anything?
>
> Shouldn't be
>
> - line->req->label, line);
> + label, line);

It should, I badly copy-pasted Kent's correct code. Thanks, I fixed it in tree.

Bart

>
> ?
>
> > + if (ret) {
> > + free_irq_label(label);
> > return ret;
> > + }
>
> --
> With Best Regards,
> Andy Shevchenko
>
>