backlight: check null deference of name when device is registered

Started by Jingoo Han on Jan 4, 2013 · 6 messages from 3 authors · Last reply Jan 8, 2013

Subsystems (3)

Maintainers (5)

Show 1 more
STALE5057d REVIEWED · 1 review, 0 by maintainers

Status: Reviewed.

1 review trailer.

open the first message on its own page

Jingoo Han

Message details and related content
Open message details and related content
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 <redacted>
Signed-off-by: Jingoo Han <redacted>
---
 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

Andrew Morton replied to Jingoo Han

Message details and related content
Open message details and related content
On Fri, 04 Jan 2013 17:29:11 +0900
Jingoo Han [off-list ref] wrote:
quoted hunk
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?

Jingoo Han replied to Andrew Morton

Message details and related content
Open message details and related content
On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote
On Fri, 04 Jan 2013 17:29:11 +0900
Jingoo Han [off-list ref] wrote:
quoted
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

Andrew Morton replied to Jingoo Han

Message details and related content
Open message details and related content
On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [off-list ref] wrote:

On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote
quoted
On Fri, 04 Jan 2013 17:29:11 +0900
Jingoo Han [off-list ref] wrote:
quoted
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.
quoted
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.

Jingoo Han replied to Andrew Morton

Message details and related content
Open message details and related content
On Tuesday, January 08, 2013 10:36 AM, Andrew Morton wrote
On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [off-list ref] wrote:
quoted
On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote
quoted
On Fri, 04 Jan 2013 17:29:11 +0900
Jingoo Han [off-list ref] wrote:
quoted
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.
quoted
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");
+

4 more quoted lines

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





  

    

      
      devendra.aaru      replied to Andrew Morton      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
On Mon, Jan 7, 2013 at 8:35 PM, Andrew Morton [off-list ref] wrote:
On Tue, 08 Jan 2013 10:25:35 +0900 Jingoo Han [off-list ref] wrote:

quoted
On Tuesday, January 08, 2013 9:02 AM, Andrew Morton wrote
quoted
On Fri, 04 Jan 2013 17:29:11 +0900
Jingoo Han [off-list ref] wrote:

quoted
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.

quoted
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");
+
5 more quoted lines
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);










    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread / next series-diff position

          
kprevious message in thread / previous series-diff position

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help