linux-kernel - Re: [PATCH] led: core: Fix race on software blink cancellation

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: <d078e26a-6250-72c5-b9ba-28d8787fcd51@gmail.com>
Date:   Fri, 9 Feb 2018 22:35:00 +0100
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Pavel Machek <pavel@....cz>
Cc:     linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org,
        craig.mcqueen@...errange.com.au,
        Hans de Goede <hdegoede@...hat.com>,
        Sakari Ailus <sakari.ailus@...ux.intel.com>,
        Matthieu CASTET <matthieu.castet@...rot.com>,
        Dan Murphy <dmurphy@...com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Ben Whitten <ben.whitten@...il.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andrew Lunn <andrew@...n.ch>,
        Alan Mizrahi <alan@...rahi.com.ve>,
        Vadim Pasternak <vadimp@...lanox.com>,
        David Lin <dtwlin@...gle.com>, Joel Stanley <joel@....id.au>,
        Cédric Le Goater <clg@...d.org>,
        Willy Tarreau <w@....eu>, Andrew Jeffery <andrew@...id.au>,
        Javier Martinez Canillas <javier@...hile0.org>,
        Colin King <colin.king@...onical.com>
Subject: Re: [PATCH] led: core: Fix race on software blink cancellation

On 02/08/2018 10:50 PM, Pavel Machek wrote:
> Hi!
> 
>> Any comments? I'd like to have some acks before applying
>> this patch.
> 
> I can't say I like it.

Please point out the bits you don't like and spot any risks.

>>> Commit d23a22a74fde ("leds: delay led_set_brightness if stopping soft-blink")
>>> made a modifications to the LED core allowing for led_set_brightness() to be
>>> called from hard-irq context when soft blink is being handled in soft-irq.
>>>
>>> Since that time LED core has undergone modifications related to addition of
>>> generic support for delegating brightness setting to a workqueue as well as
>>> subsequent fixes for blink setting use cases.
>>>
>>> After that the LED core code became hard to maintain and analyze, especially
>>> due to the imposed hard-irq context compatibility. It also turned out that
>>> in some cases a LED remained off after executing following sequence of commands:
>>>
>>> 1. echo timer > trigger
>>> 2. echo 0 > brightness
>>> 3. echo 100 > brightness
>>>
>>> The reason was LED_BLINK_DISABLE operation delegated to a set_brightness_work,
>>> triggered in 2., which was handled after 3. took effect.
> 
> Could we wait for delayed work to be done before setting the
> brightness to fix this one?

Without mutually exclusive section you can always be preempted
after wait test succeeds and before requesting new brightness.
Then the other process can jump in, request other brightness
(and change it in struct led_classdev via led_set_brightness_nosleep()),
which causes we end up with non deterministic LED class device
state after that.

Every solution without mutually exclusive section is prone to races
here.

-- 
Best regards,
Jacek Anaszewski