backlight_register_device can oops if name is null ?

Hello,

while reading through the backlight_register_device function, there
seems to be a problem if the new object name is null, the pr_debug
print goes and dereferences a null pointer causing an oops,

the print should be removed or the check must be placed to ensure that
name is not null,

any ideas?

On Tuesday, January 01, 2013 6:05 AM, Devendra Naga wrote

Hello,

while reading through the backlight_register_device function, there

backlight_register_device -> backlight_device_register

seems to be a problem if the new object name is null, the pr_debug
print goes and dereferences a null pointer causing an oops,

the print should be removed or the check must be placed to ensure that
name is not null,

Hi,

If name is null in backlight_device_register(),
pr_debug() will work properly and won't cause an oops.

However, dev_set_name() will cause oops, instead of pr_debug().
In this case, NULL check would be better.


Best regards,
Jingoo Han

any ideas?
--
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

Hello,

Thanks :)

On Tue, Jan 1, 2013 at 7:44 PM, Jingoo Han [off-list ref] wrote:

On Tuesday, January 01, 2013 6:05 AM, Devendra Naga wrote

quoted

Hello,

while reading through the backlight_register_device function, there

backlight_register_device -> backlight_device_register

i am sorry i would have really written correctly,

quoted

seems to be a problem if the new object name is null, the pr_debug
print goes and dereferences a null pointer causing an oops,

the print should be removed or the check must be placed to ensure that
name is not null,

Hi,

If name is null in backlight_device_register(),
pr_debug() will work properly and won't cause an oops.

so if i understand the code pr_debug code, i see a printk macro assignment,

is it something like printk or a wrapper macro around printk are
making sure that the strings are non null ?

i am sorry to ask this as i dont have proper tags in vim ubuntu, its
not allowing me to a tag list when i do in fedora :(,

However, dev_set_name() will cause oops, instead of pr_debug().
In this case, NULL check would be better.

the documentation of the backlight_device_register says that "name
must be same as the name of the respected frame buffer device", but do
we really add a check for the null to do dev_set_name? or fail the
registering ?

Best regards,
Jingoo Han

quoted

any ideas?
--
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

On Thursday, January 03, 2013 1:41 AM, Devendra Naga wrote"

On Tue, Jan 1, 2013 at 7:44 PM, Jingoo Han [off-list ref] wrote:

quoted

On Tuesday, January 01, 2013 6:05 AM, Devendra Naga wrote

quoted

Hello,

while reading through the backlight_register_device function, there

backlight_register_device -> backlight_device_register

i am sorry i would have really written correctly,

quoted

quoted

seems to be a problem if the new object name is null, the pr_debug
print goes and dereferences a null pointer causing an oops,

the print should be removed or the check must be placed to ensure that
name is not null,

Hi,

If name is null in backlight_device_register(),
pr_debug() will work properly and won't cause an oops.

so if i understand the code pr_debug code, i see a printk macro assignment,

is it something like printk or a wrapper macro around printk are
making sure that the strings are non null ?

Sorry, I don't know how printk deals this null.

But I tested null by using ams369fg06 driver as below:

-	bd = backlight_device_register("ams369fg06-bl", &spi->dev, lcd,
+	bd = backlight_device_register(NULL, &spi->dev, lcd,

Also, I replaced pr_debug with printk as below:

-	pr_debug("backlight_device_register: name=%s\n", name);
+	printk("backlight_device_register: name=%s\n", name);

In this case, printk message is as below:

backlight_device_register: name=(null)

i am sorry to ask this as i dont have proper tags in vim ubuntu, its
not allowing me to a tag list when i do in fedora :(,

quoted

However, dev_set_name() will cause oops, instead of pr_debug().
In this case, NULL check would be better.

the documentation of the backlight_device_register says that "name
must be same as the name of the respected frame buffer device", but do
we really add a check for the null to do dev_set_name? or fail the
registering ?

"name must be same as the name of the respected frame buffer device" is
not correct. Most backlight drivers are using their driver name as 'name'.

Anyway, in my opinion, deference to null does not seem to happen.
Now, backlight_device_register() calls are using 'name' properly. 


However, if a check for the null is added, it will be added to
backlight_device_register() as below:

@@ -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(".....");
+               return -EINVAL
+       }
+

quoted

Best regards,
Jingoo Han

quoted

any ideas?
--
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

quoted

quoted

so if i understand the code pr_debug code, i see a printk macro assignment,

is it something like printk or a wrapper macro around printk are
making sure that the strings are non null ?

Sorry, I don't know how printk deals this null.

But I tested null by using ams369fg06 driver as below:

-       bd = backlight_device_register("ams369fg06-bl", &spi->dev, lcd,
+       bd = backlight_device_register(NULL, &spi->dev, lcd,

Also, I replaced pr_debug with printk as below:

-       pr_debug("backlight_device_register: name=%s\n", name);
+       printk("backlight_device_register: name=%s\n", name);

In this case, printk message is as below:

backlight_device_register: name=(null)

quoted

wow, you tested the code, i would have done it with a single module,
great and thanks for lot of information you are putting .

This is what exactly i was thinking, thanks for making a patch too.

If you want to submit it then please take my

Acked-By: Devendra Naga <redacted>