| Message ID | 20220706084507.2259415-1-warthog618@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: cdev: fix null pointer dereference in linereq_free() | expand |
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.
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.
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
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.
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. >
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.
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);
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(-)