Re: [PATCH v3] leds: Introduce userspace leds driver

From: David Lechner
Date: Fri Sep 16 2016 - 11:18:34 EST


On 09/16/2016 12:51 AM, Pavel Machek wrote:
On Thu 2016-09-15 10:31:50, David Lechner wrote:
On 09/15/2016 09:54 AM, Jacek Anaszewski wrote:
Hi Pavel,

On 09/15/2016 03:08 PM, Pavel Machek wrote:
Hi!

+ if (copy_from_user(&udev->user_dev, buffer,
+ sizeof(struct uleds_user_dev))) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ if (!udev->user_dev.name[0]) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = led_classdev_register(NULL, &udev->led_cdev);
+ if (ret < 0)
+ goto out;

No sanity checking on the name -> probably a security hole. Do not
push this upstream before this is fixed.

Thanks for catching this.

David, please check if the LED name sticks to the LED class
device naming convention.

I don't think it is a good idea to enforce the LED class naming convention.
Someone may have a userspace application they want to test that has a
hard-coded name that does not follow the convention.

Umm.

Noone has applications with hardcoded names that are not possible
today, right?

And better not encourage crazy names.

Best regards,
Pavel


Here is the actual `ls /sys/class/leds` from my Raspberry Pi:

led0 pistorms:BA:red:ev3dev pistorms:BB:red:ev3dev
pistorms:BA:blue:ev3dev pistorms:BB:blue:ev3dev
pistorms:BA:green:ev3dev pistorms:BB:green:ev3dev



Suppose I want to use uleds on my desktop to simulate my Raspberry Pi. If we restrict the name to the LEDs class convention of device:color:function, then I can't do this. led0 does not follow the convention at all. The other do follow the convention, but only if we allow that the device portion of the name can also include ':'.

It's too late, the crazy names already exist.