# \[PATCH\] backlight: check null deference of name when device is registered - Subsystems: FRAMEBUFFER LAYER, BACKLIGHT CLASS/SUBSYSTEM, THE REST - Maintainers: Helge Deller, Lee Jones, Daniel Thompson, Jingoo Han, Linus Torvalds - Status: REVIEWED ยท 1 review, 0 by maintainers (Devendra Naga) - Reviewed. - 1 review trailer. - List: linux-fbdev - 6 messages, 3 authors - Page: https://ratatoskr.run/linux-fbdev/2013/01/3978338/t ## \[PATCH\] backlight: check null deference of name when device is registered - From: Jingoo Han \ - Date: 2013-01-04 08:29:17 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3978338.md ```text NULL deference of name is checked when device is registered. If the name is null, it will cause a kernel oops in dev_set_name(). Acked-by: Devendra Naga Signed-off-by: Jingoo Han --- drivers/video/backlight/backlight.c | 5 +++++ drivers/video/backlight/lcd.c | 5 +++++ 2 files changed, 10 insertions(+), 0 deletions(-) diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c index 345f666..f2db904 100644 --- a/drivers/video/backlight/backlight.c +++ b/drivers/video/backlight/backlight.c @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, struct backlight_device *new_bd; int rc; + if (name = NULL) { + pr_err("backlight name is null\n"); + return ERR_PTR(-EINVAL); + } + pr_debug("backlight_device_register: name=%s\n", name); new_bd = kzalloc(sizeof(struct backlight_device), GFP_KERNEL); diff --git a/drivers/video/backlight/lcd.c b/drivers/video/backlight/lcd.c index 34fb6bd..3d0ac0c 100644 --- a/drivers/video/backlight/lcd.c +++ b/drivers/video/backlight/lcd.c @@ -207,6 +207,11 @@ struct lcd_device *lcd_device_register(const char *name, struct device *parent, struct lcd_device *new_ld; int rc; + if (name = NULL) { + pr_err("lcd name is null\n"); + return ERR_PTR(-EINVAL); + } + pr_debug("lcd_device_register: name=%s\n", name); new_ld = kzalloc(sizeof(struct lcd_device), GFP_KERNEL); -- 1.7.2.5 ``` ## Re: \[PATCH\] backlight: check null deference of name when device is registered - From: Andrew Morton \ - Date: 2013-01-08 00:01:41 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3979627.md ```text On Fri, 04 Jan 2013 17:29:11 +0900 Jingoo Han [ref] wrote: > NULL deference of name is checked when device is registered. > If the name is null, it will cause a kernel oops in dev_set_name(). > > ... > > --- a/drivers/video/backlight/backlight.c > +++ b/drivers/video/backlight/backlight.c > @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, > struct backlight_device *new_bd; > int rc; > > + if (name = NULL) { > + pr_err("backlight name is null\n"); > + return ERR_PTR(-EINVAL); > + } > + > pr_debug("backlight_device_register: name=%s\n", name); I don't understand this. Is there some driver which is calling these functions with name=NULL? If so, which one(s)? If "no" then why don't we declare that "passing name=NULL is a bug" and leave the code as-is? ``` ## Re: \[PATCH\] backlight: check null deference of name when device is registered - From: Jingoo Han \ - Date: 2013-01-08 01:25:39 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3979642.md ```text On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote > On Fri, 04 Jan 2013 17:29:11 +0900 > Jingoo Han [ref] wrote: > > > NULL deference of name is checked when device is registered. > > If the name is null, it will cause a kernel oops in dev_set_name(). > > > > ... > > > > --- a/drivers/video/backlight/backlight.c > > +++ b/drivers/video/backlight/backlight.c > > @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, > > struct backlight_device *new_bd; > > int rc; > > > > + if (name = NULL) { > > + pr_err("backlight name is null\n"); > > + return ERR_PTR(-EINVAL); > > + } > > + > > pr_debug("backlight_device_register: name=%s\n", name); > > I don't understand this. > > Is there some driver which is calling these functions with name=NULL? > If so, which one(s)? No, there is no one. > > If "no" then why don't we declare that "passing name=NULL is a bug" and > leave the code as-is? Do you mean following? + if (name = NULL) + pr_err("passing name=NULL is a bug"); + pr_debug("backlight_device_register: name=%s\n", name); Best regards, Jingoo Han ``` ## Re: \[PATCH\] backlight: check null deference of name when device is registered - From: Andrew Morton \ - Date: 2013-01-08 01:34:20 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3979645.md ```text On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [ref] wrote: > On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote > > On Fri, 04 Jan 2013 17:29:11 +0900 > > Jingoo Han [ref] wrote: > > > > > NULL deference of name is checked when device is registered. > > > If the name is null, it will cause a kernel oops in dev_set_name(). > > > > > > ... > > > > > > --- a/drivers/video/backlight/backlight.c > > > +++ b/drivers/video/backlight/backlight.c > > > @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, > > > struct backlight_device *new_bd; > > > int rc; > > > > > > + if (name = NULL) { > > > + pr_err("backlight name is null\n"); > > > + return ERR_PTR(-EINVAL); > > > + } > > > + > > > pr_debug("backlight_device_register: name=%s\n", name); > > > > I don't understand this. > > > > Is there some driver which is calling these functions with name=NULL? > > If so, which one(s)? > > No, there is no one. > > > > > If "no" then why don't we declare that "passing name=NULL is a bug" and > > leave the code as-is? > > Do you mean following? > > + if (name = NULL) > + pr_err("passing name=NULL is a bug"); > + > pr_debug("backlight_device_register: name=%s\n", name); Nope; I'm suggesting we leave the code alone. If someone passes in NULL they will get a nice oops and their bug will then get fixed. ``` ## Re: \[PATCH\] backlight: check null deference of name when device is registered - From: Jingoo Han \ - Date: 2013-01-08 01:39:44 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3979647.md ```text On Tuesday, January 08, 2013 10:36 AM, Andrew Morton wrote > On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [ref] wrote: > > > On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote > > > On Fri, 04 Jan 2013 17:29:11 +0900 > > > Jingoo Han [ref] wrote: > > > > > > > NULL deference of name is checked when device is registered. > > > > If the name is null, it will cause a kernel oops in dev_set_name(). > > > > > > > > ... > > > > > > > > --- a/drivers/video/backlight/backlight.c > > > > +++ b/drivers/video/backlight/backlight.c > > > > @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, > > > > struct backlight_device *new_bd; > > > > int rc; > > > > > > > > + if (name = NULL) { > > > > + pr_err("backlight name is null\n"); > > > > + return ERR_PTR(-EINVAL); > > > > + } > > > > + > > > > pr_debug("backlight_device_register: name=%s\n", name); > > > > > > I don't understand this. > > > > > > Is there some driver which is calling these functions with name=NULL? > > > If so, which one(s)? > > > > No, there is no one. > > > > > > > > If "no" then why don't we declare that "passing name=NULL is a bug" and > > > leave the code as-is? > > > > Do you mean following? > > > > + if (name = NULL) > > + pr_err("passing name=NULL is a bug"); > > + > > pr_debug("backlight_device_register: name=%s\n", name); > > Nope; I'm suggesting we leave the code alone. If someone passes in > NULL they will get a nice oops and their bug will then get fixed. I see. I agree with you. Please, abandon this patch. Best regards, Jingoo Han > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ``` ## Re: \[PATCH\] backlight: check null deference of name when device is registered - From: devendra.aaru \ - Date: 2013-01-08 04:29:00 - Message: https://ratatoskr.run/linux-fbdev/2013/01/3979732.md ```text On Mon, Jan 7, 2013 at 8:35 PM, Andrew Morton [ref] wrote: > On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [ref] wrote: > >> On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote >> > On Fri, 04 Jan 2013 17:29:11 +0900 >> > Jingoo Han [ref] wrote: >> > >> > > NULL deference of name is checked when device is registered. >> > > If the name is null, it will cause a kernel oops in dev_set_name(). >> > > >> > > ... >> > > >> > > --- a/drivers/video/backlight/backlight.c >> > > +++ b/drivers/video/backlight/backlight.c >> > > @@ -292,6 +292,11 @@ struct backlight_device *backlight_device_register(const char *name, >> > > struct backlight_device *new_bd; >> > > int rc; >> > > >> > > + if (name = NULL) { >> > > + pr_err("backlight name is null\n"); >> > > + return ERR_PTR(-EINVAL); >> > > + } >> > > + >> > > pr_debug("backlight_device_register: name=%s\n", name); >> > >> > I don't understand this. >> > >> > Is there some driver which is calling these functions with name=NULL? >> > If so, which one(s)? >> >> No, there is no one. >> >> > >> > If "no" then why don't we declare that "passing name=NULL is a bug" and >> > leave the code as-is? >> >> Do you mean following? >> >> + if (name = NULL) >> + pr_err("passing name=NULL is a bug"); >> + >> pr_debug("backlight_device_register: name=%s\n", name); > > Nope; I'm suggesting we leave the code alone. If someone passes in > NULL they will get a nice oops and their bug will then get fixed. > We still fail the probe in the patch Jingoo Han sent, anyways if i catch your point correctly is this the lines you wanted? + /** + * BUG if the name of the backlight device + * is a NULL + */ + BUG_ON(!name); ```