* [PATCH] leds: fix a potential NULL pointer dereference @ 2019-03-09 6:04 Kangjie Lu 2019-03-10 20:27 ` Jacek Anaszewski 0 siblings, 1 reply; 8+ messages in thread From: Kangjie Lu @ 2019-03-09 6:04 UTC (permalink / raw) To: kjlu Cc: pakki001, Riku Voipio, Jacek Anaszewski, Pavel Machek, linux-leds, linux-kernel In case of_match_device cannot find a match, the fixes returns -EINVAL to avoid NULL pointer dereference. Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- drivers/leds/leds-pca9532.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c index 7fea18b0c15d..4b0335591728 100644 --- a/drivers/leds/leds-pca9532.c +++ b/drivers/leds/leds-pca9532.c @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, const struct i2c_device_id *id) { int devid; + const struct of_device_id *of_id; struct pca9532_data *data = i2c_get_clientdata(client); struct pca9532_platform_data *pca9532_pdata = dev_get_platdata(&client->dev); @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, dev_err(&client->dev, "no platform data\n"); return -EINVAL; } - devid = (int)(uintptr_t)of_match_device( - of_pca9532_leds_match, &client->dev)->data; + of_id = of_match_device(of_pca9532_leds_match, + &client->dev); + if (unlikely(!of_id)) + return -EINVAL; + devid = (int)of_id->data; } else { devid = id->driver_data; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-09 6:04 [PATCH] leds: fix a potential NULL pointer dereference Kangjie Lu @ 2019-03-10 20:27 ` Jacek Anaszewski 2019-03-11 10:09 ` Enrico Weigelt, metux IT consult 2019-03-31 9:06 ` Geert Uytterhoeven 0 siblings, 2 replies; 8+ messages in thread From: Jacek Anaszewski @ 2019-03-10 20:27 UTC (permalink / raw) To: Kangjie Lu; +Cc: pakki001, Riku Voipio, Pavel Machek, linux-leds, linux-kernel Hi Kangjie, Thank you for the patch. On 3/9/19 7:04 AM, Kangjie Lu wrote: > In case of_match_device cannot find a match, the fixes returns > -EINVAL to avoid NULL pointer dereference. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/leds/leds-pca9532.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c > index 7fea18b0c15d..4b0335591728 100644 > --- a/drivers/leds/leds-pca9532.c > +++ b/drivers/leds/leds-pca9532.c > @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > int devid; > + const struct of_device_id *of_id; > struct pca9532_data *data = i2c_get_clientdata(client); > struct pca9532_platform_data *pca9532_pdata = > dev_get_platdata(&client->dev); > @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, > dev_err(&client->dev, "no platform data\n"); > return -EINVAL; > } > - devid = (int)(uintptr_t)of_match_device( > - of_pca9532_leds_match, &client->dev)->data; > + of_id = of_match_device(of_pca9532_leds_match, > + &client->dev); > + if (unlikely(!of_id)) > + return -EINVAL; > + devid = (int)of_id->data; > } else { > devid = id->driver_data; > } Applied to the for-5.2 branch of linux-leds.git. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-10 20:27 ` Jacek Anaszewski @ 2019-03-11 10:09 ` Enrico Weigelt, metux IT consult 2019-03-31 9:06 ` Geert Uytterhoeven 1 sibling, 0 replies; 8+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-03-11 10:09 UTC (permalink / raw) To: Jacek Anaszewski, Kangjie Lu Cc: pakki001, Riku Voipio, Pavel Machek, linux-leds, linux-kernel On 10.03.19 21:27, Jacek Anaszewski wrote: > Hi Kangjie, > > Thank you for the patch. > > On 3/9/19 7:04 AM, Kangjie Lu wrote: >> In case of_match_device cannot find a match, the fixes returns >> -EINVAL to avoid NULL pointer dereference. >> >> Signed-off-by: Kangjie Lu <kjlu@umn.edu> >> --- >> drivers/leds/leds-pca9532.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c >> index 7fea18b0c15d..4b0335591728 100644 >> --- a/drivers/leds/leds-pca9532.c >> +++ b/drivers/leds/leds-pca9532.c >> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, >> const struct i2c_device_id *id) >> { >> int devid; >> + const struct of_device_id *of_id; Looks like an indention mismatch that might call for the Great White Handkerchief ;-) --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-10 20:27 ` Jacek Anaszewski 2019-03-11 10:09 ` Enrico Weigelt, metux IT consult @ 2019-03-31 9:06 ` Geert Uytterhoeven 2019-03-31 10:52 ` Jacek Anaszewski 2019-03-31 11:01 ` Jacek Anaszewski 1 sibling, 2 replies; 8+ messages in thread From: Geert Uytterhoeven @ 2019-03-31 9:06 UTC (permalink / raw) To: Jacek Anaszewski Cc: Kangjie Lu, pakki001, Riku Voipio, Pavel Machek, linux-leds, Linux Kernel Mailing List Hi Jacek, On Sun, Mar 10, 2019 at 9:40 PM Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > On 3/9/19 7:04 AM, Kangjie Lu wrote: > > In case of_match_device cannot find a match, the fixes returns > > -EINVAL to avoid NULL pointer dereference. > > > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > > --- > > drivers/leds/leds-pca9532.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c > > index 7fea18b0c15d..4b0335591728 100644 > > --- a/drivers/leds/leds-pca9532.c > > +++ b/drivers/leds/leds-pca9532.c > > @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, > > const struct i2c_device_id *id) > > { > > int devid; > > + const struct of_device_id *of_id; > > struct pca9532_data *data = i2c_get_clientdata(client); > > struct pca9532_platform_data *pca9532_pdata = > > dev_get_platdata(&client->dev); > > @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, > > dev_err(&client->dev, "no platform data\n"); > > return -EINVAL; > > } > > - devid = (int)(uintptr_t)of_match_device( > > - of_pca9532_leds_match, &client->dev)->data; > > + of_id = of_match_device(of_pca9532_leds_match, > > + &client->dev); > > + if (unlikely(!of_id)) Use of unlikey() is frowned upon. Moreover, this cannot happen, as pca9532_of_populate_pdata() already contains a similar check. Kangjie: please stop submitting patches for missing checks, without investigating if the failures can actually happen. Thanks! > > + return -EINVAL; > > + devid = (int)of_id->data; > > } else { > > devid = id->driver_data; > > } > > > Applied to the for-5.2 branch of linux-leds.git. And also as a fix for v5.1... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-31 9:06 ` Geert Uytterhoeven @ 2019-03-31 10:52 ` Jacek Anaszewski 2019-03-31 11:01 ` Jacek Anaszewski 1 sibling, 0 replies; 8+ messages in thread From: Jacek Anaszewski @ 2019-03-31 10:52 UTC (permalink / raw) To: Linus Torvalds Cc: Geert Uytterhoeven, Kangjie Lu, pakki001, Riku Voipio, Pavel Machek, linux-leds, Linux Kernel Mailing List Hi Linus, Yesterday I submitted pull request [0] containing this patch, but the patch turns out to be pointless. Since I see my pull request merged on top of mainline trunk I suspect it will not be a problem to drop the patch or even whole pull request. In the latter case I'll re-submit it for -rc4. Sorry for the confusion. Best regards, Jacek Anaszewski On 3/31/19 11:06 AM, Geert Uytterhoeven wrote: > Hi Jacek, > > On Sun, Mar 10, 2019 at 9:40 PM Jacek Anaszewski > <jacek.anaszewski@gmail.com> wrote: >> On 3/9/19 7:04 AM, Kangjie Lu wrote: >>> In case of_match_device cannot find a match, the fixes returns >>> -EINVAL to avoid NULL pointer dereference. >>> >>> Signed-off-by: Kangjie Lu <kjlu@umn.edu> >>> --- >>> drivers/leds/leds-pca9532.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c >>> index 7fea18b0c15d..4b0335591728 100644 >>> --- a/drivers/leds/leds-pca9532.c >>> +++ b/drivers/leds/leds-pca9532.c >>> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, >>> const struct i2c_device_id *id) >>> { >>> int devid; >>> + const struct of_device_id *of_id; >>> struct pca9532_data *data = i2c_get_clientdata(client); >>> struct pca9532_platform_data *pca9532_pdata = >>> dev_get_platdata(&client->dev); >>> @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, >>> dev_err(&client->dev, "no platform data\n"); >>> return -EINVAL; >>> } >>> - devid = (int)(uintptr_t)of_match_device( >>> - of_pca9532_leds_match, &client->dev)->data; >>> + of_id = of_match_device(of_pca9532_leds_match, >>> + &client->dev); >>> + if (unlikely(!of_id)) > > Use of unlikey() is frowned upon. > Moreover, this cannot happen, as pca9532_of_populate_pdata() already > contains a similar check. > > Kangjie: please stop submitting patches for missing checks, without > investigating if the failures can actually happen. Thanks! > >>> + return -EINVAL; >>> + devid = (int)of_id->data; >>> } else { >>> devid = id->driver_data; >>> } >> >> >> Applied to the for-5.2 branch of linux-leds.git. > > And also as a fix for v5.1... > > Gr{oetje,eeting}s, > > Geert > [0] https://lkml.org/lkml/2019/3/30/222 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-31 9:06 ` Geert Uytterhoeven 2019-03-31 10:52 ` Jacek Anaszewski @ 2019-03-31 11:01 ` Jacek Anaszewski 2019-04-01 7:08 ` Geert Uytterhoeven 1 sibling, 1 reply; 8+ messages in thread From: Jacek Anaszewski @ 2019-03-31 11:01 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Kangjie Lu, pakki001, Riku Voipio, Pavel Machek, linux-leds, Linux Kernel Mailing List Hi Geert, Thank you for the notification. On 3/31/19 11:06 AM, Geert Uytterhoeven wrote: > Hi Jacek, > > On Sun, Mar 10, 2019 at 9:40 PM Jacek Anaszewski > <jacek.anaszewski@gmail.com> wrote: >> On 3/9/19 7:04 AM, Kangjie Lu wrote: >>> In case of_match_device cannot find a match, the fixes returns >>> -EINVAL to avoid NULL pointer dereference. >>> >>> Signed-off-by: Kangjie Lu <kjlu@umn.edu> >>> --- >>> drivers/leds/leds-pca9532.c | 8 ++++++-- >>> 1 file changed, 6 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c >>> index 7fea18b0c15d..4b0335591728 100644 >>> --- a/drivers/leds/leds-pca9532.c >>> +++ b/drivers/leds/leds-pca9532.c >>> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, >>> const struct i2c_device_id *id) >>> { >>> int devid; >>> + const struct of_device_id *of_id; >>> struct pca9532_data *data = i2c_get_clientdata(client); >>> struct pca9532_platform_data *pca9532_pdata = >>> dev_get_platdata(&client->dev); >>> @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, >>> dev_err(&client->dev, "no platform data\n"); >>> return -EINVAL; >>> } >>> - devid = (int)(uintptr_t)of_match_device( >>> - of_pca9532_leds_match, &client->dev)->data; >>> + of_id = of_match_device(of_pca9532_leds_match, >>> + &client->dev); >>> + if (unlikely(!of_id)) > > Use of unlikey() is frowned upon. What do you mean? Can you give some reference? > Moreover, this cannot happen, as pca9532_of_populate_pdata() already > contains a similar check. Right, I assumed this fixes a real problem and didn't spent too much time investigating the whole context.. Lesson for the future. > Kangjie: please stop submitting patches for missing checks, without > investigating if the failures can actually happen. Thanks! > >>> + return -EINVAL; >>> + devid = (int)of_id->data; >>> } else { >>> devid = id->driver_data; >>> } >> >> >> Applied to the for-5.2 branch of linux-leds.git. > > And also as a fix for v5.1... Yes, but it had been in linux-next for almost two weeks before that. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-03-31 11:01 ` Jacek Anaszewski @ 2019-04-01 7:08 ` Geert Uytterhoeven 2019-04-02 18:48 ` Jacek Anaszewski 0 siblings, 1 reply; 8+ messages in thread From: Geert Uytterhoeven @ 2019-04-01 7:08 UTC (permalink / raw) To: Jacek Anaszewski Cc: Kangjie Lu, pakki001, Riku Voipio, Pavel Machek, linux-leds, Linux Kernel Mailing List Hi Jacek, On Sun, Mar 31, 2019 at 1:01 PM Jacek Anaszewski <jacek.anaszewski@gmail.com> wrote: > On 3/31/19 11:06 AM, Geert Uytterhoeven wrote: > On Sun, Mar 10, 2019 at 9:40 PM Jacek Anaszewski > > <jacek.anaszewski@gmail.com> wrote: > >> On 3/9/19 7:04 AM, Kangjie Lu wrote: > >>> In case of_match_device cannot find a match, the fixes returns > >>> -EINVAL to avoid NULL pointer dereference. > >>> > >>> Signed-off-by: Kangjie Lu <kjlu@umn.edu> > >>> --- > >>> drivers/leds/leds-pca9532.c | 8 ++++++-- > >>> 1 file changed, 6 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c > >>> index 7fea18b0c15d..4b0335591728 100644 > >>> --- a/drivers/leds/leds-pca9532.c > >>> +++ b/drivers/leds/leds-pca9532.c > >>> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, > >>> const struct i2c_device_id *id) > >>> { > >>> int devid; > >>> + const struct of_device_id *of_id; > >>> struct pca9532_data *data = i2c_get_clientdata(client); > >>> struct pca9532_platform_data *pca9532_pdata = > >>> dev_get_platdata(&client->dev); > >>> @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, > >>> dev_err(&client->dev, "no platform data\n"); > >>> return -EINVAL; > >>> } > >>> - devid = (int)(uintptr_t)of_match_device( > >>> - of_pca9532_leds_match, &client->dev)->data; > >>> + of_id = of_match_device(of_pca9532_leds_match, > >>> + &client->dev); > >>> + if (unlikely(!of_id)) > > > > Use of unlikey() is frowned upon. > > What do you mean? Can you give some reference? I have more memories of this being discussed, but I could find only https://lwn.net/Articles/420019/ It may be useful for some heavily used core code, but not in most drivers or uncritical code like probe paths, due to: - many people getting it wrong, - usually it doesn't make any difference at all. > >> Applied to the for-5.2 branch of linux-leds.git. > > > > And also as a fix for v5.1... > > Yes, but it had been in linux-next for almost two weeks before that. Sorry, I only noticed when it got upstream. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] leds: fix a potential NULL pointer dereference 2019-04-01 7:08 ` Geert Uytterhoeven @ 2019-04-02 18:48 ` Jacek Anaszewski 0 siblings, 0 replies; 8+ messages in thread From: Jacek Anaszewski @ 2019-04-02 18:48 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Kangjie Lu, pakki001, Riku Voipio, Pavel Machek, linux-leds, Linux Kernel Mailing List Hi Geert, On 4/1/19 9:08 AM, Geert Uytterhoeven wrote: > Hi Jacek, > > On Sun, Mar 31, 2019 at 1:01 PM Jacek Anaszewski > <jacek.anaszewski@gmail.com> wrote: >> On 3/31/19 11:06 AM, Geert Uytterhoeven wrote: >> On Sun, Mar 10, 2019 at 9:40 PM Jacek Anaszewski >>> <jacek.anaszewski@gmail.com> wrote: >>>> On 3/9/19 7:04 AM, Kangjie Lu wrote: >>>>> In case of_match_device cannot find a match, the fixes returns >>>>> -EINVAL to avoid NULL pointer dereference. >>>>> >>>>> Signed-off-by: Kangjie Lu <kjlu@umn.edu> >>>>> --- >>>>> drivers/leds/leds-pca9532.c | 8 ++++++-- >>>>> 1 file changed, 6 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/leds/leds-pca9532.c b/drivers/leds/leds-pca9532.c >>>>> index 7fea18b0c15d..4b0335591728 100644 >>>>> --- a/drivers/leds/leds-pca9532.c >>>>> +++ b/drivers/leds/leds-pca9532.c >>>>> @@ -513,6 +513,7 @@ static int pca9532_probe(struct i2c_client *client, >>>>> const struct i2c_device_id *id) >>>>> { >>>>> int devid; >>>>> + const struct of_device_id *of_id; >>>>> struct pca9532_data *data = i2c_get_clientdata(client); >>>>> struct pca9532_platform_data *pca9532_pdata = >>>>> dev_get_platdata(&client->dev); >>>>> @@ -528,8 +529,11 @@ static int pca9532_probe(struct i2c_client *client, >>>>> dev_err(&client->dev, "no platform data\n"); >>>>> return -EINVAL; >>>>> } >>>>> - devid = (int)(uintptr_t)of_match_device( >>>>> - of_pca9532_leds_match, &client->dev)->data; >>>>> + of_id = of_match_device(of_pca9532_leds_match, >>>>> + &client->dev); >>>>> + if (unlikely(!of_id)) >>> >>> Use of unlikey() is frowned upon. >> >> What do you mean? Can you give some reference? > > I have more memories of this being discussed, but I could find only > https://lwn.net/Articles/420019/ Thanks! > It may be useful for some heavily used core code, but not in most drivers > or uncritical code like probe paths, due to: > - many people getting it wrong, > - usually it doesn't make any difference at all. > >>>> Applied to the for-5.2 branch of linux-leds.git. >>> >>> And also as a fix for v5.1... >> >> Yes, but it had been in linux-next for almost two weeks before that. > > Sorry, I only noticed when it got upstream. No problem. -- Best regards, Jacek Anaszewski ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-04-02 18:48 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-09 6:04 [PATCH] leds: fix a potential NULL pointer dereference Kangjie Lu 2019-03-10 20:27 ` Jacek Anaszewski 2019-03-11 10:09 ` Enrico Weigelt, metux IT consult 2019-03-31 9:06 ` Geert Uytterhoeven 2019-03-31 10:52 ` Jacek Anaszewski 2019-03-31 11:01 ` Jacek Anaszewski 2019-04-01 7:08 ` Geert Uytterhoeven 2019-04-02 18:48 ` Jacek Anaszewski
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).