[PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Kent Gibson]

This patch fixes a kernel NULL pointer dereference that is reported by
gpio kselftests.

linereq_free() can be called as part of the cleanup of a failed request,
at which time the desc for a line may not have been determined, so it
is unsafe to dereference without a check.

Add a check prior to dereferencing the line desc.

Fixes: 2068339a6c35 ("gpiolib: cdev: Add hardware timestamp clock type")
Signed-off-by: Kent Gibson <warthog618@gmail.com>
---

I suspect the edge_detector_stop() and gpiod_free() could also be moved
inside the same desc check but, as we are late in the rc cycle, I don't
want to push my luck and have kept to the minimum change required to
address the bug.

 drivers/gpio/gpiolib-cdev.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index f5aa5f93342a..0c9a63becfef 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -1460,11 +1460,12 @@ static ssize_t linereq_read(struct file *file,
 static void linereq_free(struct linereq *lr)
 {
 	unsigned int i;
-	bool hte;
+	bool hte = false;
 
 	for (i = 0; i < lr->num_lines; i++) {
-		hte = !!test_bit(FLAG_EVENT_CLOCK_HTE,
-				 &lr->lines[i].desc->flags);
+		if (lr->lines[i].desc)
+			hte = !!test_bit(FLAG_EVENT_CLOCK_HTE,
+					 &lr->lines[i].desc->flags);
 		edge_detector_stop(&lr->lines[i], hte);
 		if (lr->lines[i].desc)
 			gpiod_free(lr->lines[i].desc);
-- 
2.37.0

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Kent Gibson]

On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> This patch fixes a kernel NULL pointer dereference that is reported by
> gpio kselftests.
> 

Should be:

Fix a kernel NULL pointer dereference reported by gpio kselftests.

Sorry - I rushed this one a bit.

Cheers,
Kent.

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Kent Gibson]

On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
> On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> > This patch fixes a kernel NULL pointer dereference that is reported by
> > gpio kselftests.
> > 
> 
> Should be:
> 
> Fix a kernel NULL pointer dereference reported by gpio kselftests.
> 
> Sorry - I rushed this one a bit.
> 

And I might not've been totally clear, but this bug is present in
v5.19-rc1 onwards (when HTE was added), up to and including rc5.

Would be good to get it fixed before v5.19 goes out the door.

Cheers,
Kent.

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Bartosz Golaszewski]

On Thu, Jul 7, 2022 at 11:00 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
> > On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> > > This patch fixes a kernel NULL pointer dereference that is reported by
> > > gpio kselftests.
> > >
> >
> > Should be:
> >
> > Fix a kernel NULL pointer dereference reported by gpio kselftests.
> >
> > Sorry - I rushed this one a bit.
> >
>
> And I might not've been totally clear, but this bug is present in
> v5.19-rc1 onwards (when HTE was added), up to and including rc5.
>
> Would be good to get it fixed before v5.19 goes out the door.
>
> Cheers,
> Kent.

Yep, figured that out. Applied and fixed the commit message, thanks!

Bart

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Kent Gibson]

On Thu, Jul 07, 2022 at 12:19:15PM +0200, Bartosz Golaszewski wrote:
> On Thu, Jul 7, 2022 at 11:00 AM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
> > > On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> > > > This patch fixes a kernel NULL pointer dereference that is reported by
> > > > gpio kselftests.
> > > >
> > >
> > > Should be:
> > >
> > > Fix a kernel NULL pointer dereference reported by gpio kselftests.
> > >
> > > Sorry - I rushed this one a bit.
> > >
> >
> > And I might not've been totally clear, but this bug is present in
> > v5.19-rc1 onwards (when HTE was added), up to and including rc5.
> >
> > Would be good to get it fixed before v5.19 goes out the door.
> >
> > Cheers,
> > Kent.
> 
> Yep, figured that out. Applied and fixed the commit message, thanks!
> 

Good to hear.  I never got around to reviewing that final HTE patch
and, while it did end up pretty close to what I expected, there are a
few things that I would've done slightly differently that I'd like to
tidy up.
And also have the HTE specific code compiled out unless CONFIG_HTE is
selected, as that is very likely to be the case for most builds.
But that can wait for v5.20.

Cheers,
Kent.

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Dipen Patel]

On 7/7/22 3:29 AM, Kent Gibson wrote:
> On Thu, Jul 07, 2022 at 12:19:15PM +0200, Bartosz Golaszewski wrote:
>> On Thu, Jul 7, 2022 at 11:00 AM Kent Gibson <warthog618@gmail.com> wrote:
>>> On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
>>>> On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
>>>>> This patch fixes a kernel NULL pointer dereference that is reported by
>>>>> gpio kselftests.
>>>>>
>>>> Should be:
>>>>
>>>> Fix a kernel NULL pointer dereference reported by gpio kselftests.
>>>>
>>>> Sorry - I rushed this one a bit.
>>>>
>>> And I might not've been totally clear, but this bug is present in
>>> v5.19-rc1 onwards (when HTE was added), up to and including rc5.
>>>
>>> Would be good to get it fixed before v5.19 goes out the door.
>>>
>>> Cheers,
>>> Kent.
>> Yep, figured that out. Applied and fixed the commit message, thanks!
>>
> Good to hear.  I never got around to reviewing that final HTE patch
> and, while it did end up pretty close to what I expected, there are a
> few things that I would've done slightly differently that I'd like to
> tidy up.
I can create another thread to address this. Let me know.
> And also have the HTE specific code compiled out unless CONFIG_HTE is
> selected, as that is very likely to be the case for most builds.
> But that can wait for v5.20.

I am assuming #ifdef CONFIG_HTE blocks around HTE blocks. I think Linus W. also

indicated that.

>
> Cheers,
> Kent.
>

Re: [PATCH] gpiolib: cdev: fix null pointer dereference in linereq_free()

[Kent Gibson]

On Fri, Jul 08, 2022 at 11:24:43AM -0700, Dipen Patel wrote:
> On 7/7/22 3:29 AM, Kent Gibson wrote:
> > On Thu, Jul 07, 2022 at 12:19:15PM +0200, Bartosz Golaszewski wrote:
> >> On Thu, Jul 7, 2022 at 11:00 AM Kent Gibson <warthog618@gmail.com> wrote:
> >>> On Wed, Jul 06, 2022 at 04:50:25PM +0800, Kent Gibson wrote:
> >>>> On Wed, Jul 06, 2022 at 04:45:07PM +0800, Kent Gibson wrote:
> >>>>> This patch fixes a kernel NULL pointer dereference that is reported by
> >>>>> gpio kselftests.
> >>>>>
> >>>> Should be:
> >>>>
> >>>> Fix a kernel NULL pointer dereference reported by gpio kselftests.
> >>>>
> >>>> Sorry - I rushed this one a bit.
> >>>>
> >>> And I might not've been totally clear, but this bug is present in
> >>> v5.19-rc1 onwards (when HTE was added), up to and including rc5.
> >>>
> >>> Would be good to get it fixed before v5.19 goes out the door.
> >>>
> >>> Cheers,
> >>> Kent.
> >> Yep, figured that out. Applied and fixed the commit message, thanks!
> >>
> > Good to hear.  I never got around to reviewing that final HTE patch
> > and, while it did end up pretty close to what I expected, there are a
> > few things that I would've done slightly differently that I'd like to
> > tidy up.
> I can create another thread to address this. Let me know.

I've already got the changes locally, so don't worry. There is nothing
critical or earth shattering in there, mainly how the hte_en flag is
passed around.  You followed the pattern provided by polarity_change,
which is only used on one occasion, when I would've followed the pattern
from line->eflags, which is used more widely.

Though I might take the changes a little further than I have to also
change the polarity_change flag to follow the eflags pattern to prevent
anyone following from falling into the same trap - still stewing on that.

The basic idea is that, particularly during reconfig, the struct line
contains the current applied state, while the desc->flags contain the
requested state.  The polarity_change flag was a bit of a lazy shortcut
to save from adding an active_low (or equivalent) flag to struct line.

> > And also have the HTE specific code compiled out unless CONFIG_HTE is
> > selected, as that is very likely to be the case for most builds.
> > But that can wait for v5.20.
> 
> I am assuming #ifdef CONFIG_HTE blocks around HTE blocks. I think Linus W. also
> 
> indicated that.
> 

Correct.

Cheers,
Kent.