linux-kernel - Re: [PATCH v4 1/3] leds: Introduce userspace leds driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-id: <0a01e033-b04a-0ffb-b694-ac55649a7623@samsung.com>
Date:   Wed, 09 Nov 2016 09:44:29 +0100
From:   Jacek Anaszewski <j.anaszewski@...sung.com>
To:     Pavel Machek <pavel@....cz>
Cc:     David Lechner <david@...hnology.com>,
        Richard Purdie <rpurdie@...ys.net>,
        linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
        Marcel Holtmann <marcel@...tmann.org>,
        Hans de Goede <hdegoede@...hat.com>
Subject: Re: [PATCH v4 1/3] leds: Introduce userspace leds driver

Hi,

On 11/09/2016 08:05 AM, Pavel Machek wrote:
> Hi!
>
>>> +struct uleds_device {
>>> +	struct uleds_user_dev	user_dev;
>>> +	struct led_classdev	led_cdev;
>>> +	struct mutex		mutex;
>>> +	enum uleds_state	state;
>>> +	wait_queue_head_t	waitq;
>>> +	unsigned char		brightness;
>>
>> I've just noticed that this is wrong, since LED subsystem
>> brightness type is enum led_brightness, i.e. int.
>> LED_FULL (255) value is a legacy enum value that can be overridden
>> by max_brightness property.
>>
>> Please submit a fix so that I could merge it with the original
>> patch before sending it upstream.
>
> Actually... perhaps you want to wait with merging the userspace driver
> till the locking is solved in the LED subsystem? Maybe I'm wrong, but
> I have feeling that userspace driver will have unusual requirements
> w.r.t. locking, and that it would be good to have that solved,
> first...

If you think about locking between led_set_brightness() and
led_update_brightness() then we have no ready solution for that.
Do you have any?

If not then we have to live with that until one is devised.
After adding the locking in brightness_show the risk of races will be
only in case of concurrent calls to led_set_brightness() and
led_update_brightness() from kernel, whereas currently there are no
such use cases.

-- 
Best regards,
Jacek Anaszewski