* [PATCH] platform/x86: intel_skl_int3472: Correct null check
@ 2021-10-08 22:46 Daniel Scally
2021-10-11 14:06 ` Hans de Goede
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Scally @ 2021-10-08 22:46 UTC (permalink / raw)
To: platform-driver-x86
Cc: Daniel Scally, Hans de Goede, Mark Gross, Andy Shevchenko
The int3472-discrete driver can enter an error path after initialising
int3472->clock.ena_gpio, but before it has registered the clock. This will
cause a NULL pointer dereference, because clkdev_drop() is not null aware.
Instead of guarding the call to skl_int3472_unregister_clock() by checking
for .ena_gpio, check specifically for the presence of the clk_lookup, which
will guarantee clkdev_create() has already been called.
Bug: https://bugzilla.kernel.org/show_bug.cgi?id=214453
Fixes: 7540599a5ef1 ("platform/x86: intel_skl_int3472: Provide skl_int3472_unregister_clock()")
Signed-off-by: Daniel Scally <djrscally@gmail.com>
---
drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
index 9fe0a2527e1c..e59d79c7e82f 100644
--- a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
+++ b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
@@ -401,7 +401,7 @@ int skl_int3472_discrete_remove(struct platform_device *pdev)
gpiod_remove_lookup_table(&int3472->gpios);
- if (int3472->clock.ena_gpio)
+ if (int3472->clock.cl)
skl_int3472_unregister_clock(int3472);
gpiod_put(int3472->clock.ena_gpio);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: intel_skl_int3472: Correct null check
2021-10-08 22:46 [PATCH] platform/x86: intel_skl_int3472: Correct null check Daniel Scally
@ 2021-10-11 14:06 ` Hans de Goede
[not found] ` <CAHp75VeRe7-CDC9PNxfa+j0JYM8OQVKUsZ=1bBDymH0ruB3szQ@mail.gmail.com>
0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2021-10-11 14:06 UTC (permalink / raw)
To: Daniel Scally, platform-driver-x86; +Cc: Mark Gross, Andy Shevchenko
Hi,
On 10/9/21 12:46 AM, Daniel Scally wrote:
> The int3472-discrete driver can enter an error path after initialising
> int3472->clock.ena_gpio, but before it has registered the clock. This will
> cause a NULL pointer dereference, because clkdev_drop() is not null aware.
> Instead of guarding the call to skl_int3472_unregister_clock() by checking
> for .ena_gpio, check specifically for the presence of the clk_lookup, which
> will guarantee clkdev_create() has already been called.
>
> Bug: https://bugzilla.kernel.org/show_bug.cgi?id=214453
> Fixes: 7540599a5ef1 ("platform/x86: intel_skl_int3472: Provide skl_int3472_unregister_clock()")
> Signed-off-by: Daniel Scally <djrscally@gmail.com>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
I will also include this in my upcoming pdx86-fixes pull-req for 5.15 .
Regards,
Hans
> ---
> drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> index 9fe0a2527e1c..e59d79c7e82f 100644
> --- a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> +++ b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> @@ -401,7 +401,7 @@ int skl_int3472_discrete_remove(struct platform_device *pdev)
>
> gpiod_remove_lookup_table(&int3472->gpios);
>
> - if (int3472->clock.ena_gpio)
> + if (int3472->clock.cl)
> skl_int3472_unregister_clock(int3472);
>
> gpiod_put(int3472->clock.ena_gpio);
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: intel_skl_int3472: Correct null check
[not found] ` <CAHp75VeRe7-CDC9PNxfa+j0JYM8OQVKUsZ=1bBDymH0ruB3szQ@mail.gmail.com>
@ 2021-10-11 22:24 ` Daniel Scally
2021-10-12 11:38 ` Hans de Goede
2021-10-12 11:37 ` Hans de Goede
1 sibling, 1 reply; 5+ messages in thread
From: Daniel Scally @ 2021-10-11 22:24 UTC (permalink / raw)
To: Andy Shevchenko, Hans de Goede
Cc: platform-driver-x86, Mark Gross, Andy Shevchenko
Hi Hans, Andy
On 11/10/2021 20:29, Andy Shevchenko wrote:
>
>
> On Monday, October 11, 2021, Hans de Goede <hdegoede@redhat.com
> <mailto:hdegoede@redhat.com>> wrote:
>
> Hi,
>
> On 10/9/21 12:46 AM, Daniel Scally wrote:
> > The int3472-discrete driver can enter an error path after
> initialising
> > int3472->clock.ena_gpio, but before it has registered the clock.
> This will
> > cause a NULL pointer dereference, because clkdev_drop() is not
> null aware.
> > Instead of guarding the call to skl_int3472_unregister_clock()
> by checking
> > for .ena_gpio, check specifically for the presence of the
> clk_lookup, which
> > will guarantee clkdev_create() has already been called.
> >
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=214453
> <https://bugzilla.kernel.org/show_bug.cgi?id=214453>
>
>
>
> Is it possible to fix this to be BugLink?
I also forgot to CC stable: my bad. I think there's a bot that picks up
things with a Fixes: tag if you do that right?
>
>
>
> > Fixes: 7540599a5ef1 ("platform/x86: intel_skl_int3472: Provide
> skl_int3472_unregister_clock()")
> > Signed-off-by: Daniel Scally <djrscally@gmail.com
> <mailto:djrscally@gmail.com>>
>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
> <https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans>
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> I will also include this in my upcoming pdx86-fixes pull-req for
> 5.15 .
>
> Regards,
>
> Hans
>
>
> > ---
> > drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git
> a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > index 9fe0a2527e1c..e59d79c7e82f 100644
> > ---
> a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > +++
> b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > @@ -401,7 +401,7 @@ int skl_int3472_discrete_remove(struct
> platform_device *pdev)
> >
> > gpiod_remove_lookup_table(&int3472->gpios);
> >
> > - if (int3472->clock.ena_gpio)
> > + if (int3472->clock.cl <http://clock.cl>)
> > skl_int3472_unregister_clock(int3472);
> >
> > gpiod_put(int3472->clock.ena_gpio);
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: intel_skl_int3472: Correct null check
[not found] ` <CAHp75VeRe7-CDC9PNxfa+j0JYM8OQVKUsZ=1bBDymH0ruB3szQ@mail.gmail.com>
2021-10-11 22:24 ` Daniel Scally
@ 2021-10-12 11:37 ` Hans de Goede
1 sibling, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-10-12 11:37 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Daniel Scally, platform-driver-x86, Mark Gross, Andy Shevchenko
Hi,
On 10/11/21 9:29 PM, Andy Shevchenko wrote:
>
>
> On Monday, October 11, 2021, Hans de Goede <hdegoede@redhat.com <mailto:hdegoede@redhat.com>> wrote:
>
> Hi,
>
> On 10/9/21 12:46 AM, Daniel Scally wrote:
> > The int3472-discrete driver can enter an error path after initialising
> > int3472->clock.ena_gpio, but before it has registered the clock. This will
> > cause a NULL pointer dereference, because clkdev_drop() is not null aware.
> > Instead of guarding the call to skl_int3472_unregister_clock() by checking
> > for .ena_gpio, check specifically for the presence of the clk_lookup, which
> > will guarantee clkdev_create() has already been called.
> >
> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=214453 <https://bugzilla.kernel.org/show_bug.cgi?id=214453>
>
>
>
> Is it possible to fix this to be BugLink?
IMHO changing Bug: to BugLink: is not worth doing a forced push to pdx86/for-next
(and pdx86/fixes) for.
Regards,
Hans
> > Fixes: 7540599a5ef1 ("platform/x86: intel_skl_int3472: Provide skl_int3472_unregister_clock()")
> > Signed-off-by: Daniel Scally <djrscally@gmail.com <mailto:djrscally@gmail.com>>
>
> Thank you for your patch, I've applied this patch to my review-hans
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans <https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans>
>
> Note it will show up in my review-hans branch once I've pushed my
> local branch there, which might take a while.
>
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
>
> I will also include this in my upcoming pdx86-fixes pull-req for 5.15 .
>
> Regards,
>
> Hans
>
>
> > ---
> > drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > index 9fe0a2527e1c..e59d79c7e82f 100644
> > --- a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > +++ b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
> > @@ -401,7 +401,7 @@ int skl_int3472_discrete_remove(struct platform_device *pdev)
> >
> > gpiod_remove_lookup_table(&int3472->gpios);
> >
> > - if (int3472->clock.ena_gpio)
> > + if (int3472->clock.cl <http://clock.cl>)
> > skl_int3472_unregister_clock(int3472);
> >
> > gpiod_put(int3472->clock.ena_gpio);
> >
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/x86: intel_skl_int3472: Correct null check
2021-10-11 22:24 ` Daniel Scally
@ 2021-10-12 11:38 ` Hans de Goede
0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-10-12 11:38 UTC (permalink / raw)
To: Daniel Scally, Andy Shevchenko
Cc: platform-driver-x86, Mark Gross, Andy Shevchenko
Hi,
On 10/12/21 12:24 AM, Daniel Scally wrote:
> Hi Hans, Andy
>
> On 11/10/2021 20:29, Andy Shevchenko wrote:
>>
>>
>> On Monday, October 11, 2021, Hans de Goede <hdegoede@redhat.com
>> <mailto:hdegoede@redhat.com>> wrote:
>>
>> Hi,
>>
>> On 10/9/21 12:46 AM, Daniel Scally wrote:
>> > The int3472-discrete driver can enter an error path after
>> initialising
>> > int3472->clock.ena_gpio, but before it has registered the clock.
>> This will
>> > cause a NULL pointer dereference, because clkdev_drop() is not
>> null aware.
>> > Instead of guarding the call to skl_int3472_unregister_clock()
>> by checking
>> > for .ena_gpio, check specifically for the presence of the
>> clk_lookup, which
>> > will guarantee clkdev_create() has already been called.
>> >
>> > Bug: https://bugzilla.kernel.org/show_bug.cgi?id=214453
>> <https://bugzilla.kernel.org/show_bug.cgi?id=214453>
>>
>>
>>
>> Is it possible to fix this to be BugLink?
>
>
> I also forgot to CC stable: my bad. I think there's a bot that picks up
> things with a Fixes: tag if you do that right?
Right, with a Fixes tag there is no strict need for a Cc: stable
(adding Cc: stable is still a good idea for pure fixes though).
Regards,
Hans
>
>>
>>
>>
>> > Fixes: 7540599a5ef1 ("platform/x86: intel_skl_int3472: Provide
>> skl_int3472_unregister_clock()")
>> > Signed-off-by: Daniel Scally <djrscally@gmail.com
>> <mailto:djrscally@gmail.com>>
>>
>> Thank you for your patch, I've applied this patch to my review-hans
>> branch:
>> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
>> <https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans>
>>
>> Note it will show up in my review-hans branch once I've pushed my
>> local branch there, which might take a while.
>>
>> Once I've run some tests on this branch the patches there will be
>> added to the platform-drivers-x86/for-next branch and eventually
>> will be included in the pdx86 pull-request to Linus for the next
>> merge-window.
>>
>> I will also include this in my upcoming pdx86-fixes pull-req for
>> 5.15 .
>>
>> Regards,
>>
>> Hans
>>
>>
>> > ---
>> > drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
>> | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git
>> a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
>> b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
>> > index 9fe0a2527e1c..e59d79c7e82f 100644
>> > ---
>> a/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
>> > +++
>> b/drivers/platform/x86/intel/int3472/intel_skl_int3472_discrete.c
>> > @@ -401,7 +401,7 @@ int skl_int3472_discrete_remove(struct
>> platform_device *pdev)
>> >
>> > gpiod_remove_lookup_table(&int3472->gpios);
>> >
>> > - if (int3472->clock.ena_gpio)
>> > + if (int3472->clock.cl <http://clock.cl>)
>> > skl_int3472_unregister_clock(int3472);
>> >
>> > gpiod_put(int3472->clock.ena_gpio);
>> >
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-12 11:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-08 22:46 [PATCH] platform/x86: intel_skl_int3472: Correct null check Daniel Scally
2021-10-11 14:06 ` Hans de Goede
[not found] ` <CAHp75VeRe7-CDC9PNxfa+j0JYM8OQVKUsZ=1bBDymH0ruB3szQ@mail.gmail.com>
2021-10-11 22:24 ` Daniel Scally
2021-10-12 11:38 ` Hans de Goede
2021-10-12 11:37 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.