Received-SPF: pass (google.com: domain of linux-kernel+bounces-131859-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240404093328.21604-1-brgl@bgdev.pl> <20240404093328.21604-3-brgl@bgdev.pl>
 <Zg7I7nYkZLcIgETq@smile.fi.intel.com>
In-Reply-To: <Zg7I7nYkZLcIgETq@smile.fi.intel.com>
From: Bartosz Golaszewski <brgl@bgdev.pl>
Date: Thu, 4 Apr 2024 18:57:39 +0200
Message-ID: <CAMRc=Me=nH0bWzd3WstJnkQYmLHN7c+c4_wq41JXzcfp5pODRg@mail.gmail.com>
Subject: Re: [PATCH v2 2/2] gpio: cdev: fix missed label sanitizing in debounce_setup()
To: Andy Shevchenko <andriy.shevchenko@intel.com>
Cc: Kent Gibson <warthog618@gmail.com>, Linus Walleij <linus.walleij@linaro.org>, 
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Alexey Dobriyan <adobriyan@gmail.com>, stable@vger.kernel.org, 
	Stefan Wahren <wahrenst@gmx.net>, Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, Apr 4, 2024 at 5:36=E2=80=AFPM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Thu, Apr 04, 2024 at 11:33:28AM +0200, Bartosz Golaszewski wrote:
> > From: Kent Gibson <warthog618@gmail.com>
> >
> > 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 =3D 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 =3D make_irq_label(line->req->label);
> > +                     if (IS_ERR(label))
> > +                             return -ENOMEM;
> > +
> >                       irqflags =3D IRQF_TRIGGER_FALLING | IRQF_TRIGGER_=
RISING;
> >                       ret =3D request_irq(irq, debounce_irq_handler, ir=
qflags,
> >                                         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 t=
ree.

Bart

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