linux-kernel - Re: [ISSUE] Memleak in LED sysfs on heavy usage

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] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 16 Sep 2016 20:49:44 +0200
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Greg KH <greg@...ah.com>,
        Jacek Anaszewski <j.anaszewski@...sung.com>
Cc:     Daniel Gorsulowski <daniel.gorsulowski@....eu>,
        "linux-leds@...r.kernel.org" <linux-leds@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [ISSUE] Memleak in LED sysfs on heavy usage

On 09/16/2016 04:39 PM, Greg KH wrote:
> On Fri, Sep 16, 2016 at 04:32:39PM +0200, Jacek Anaszewski wrote:
>> On 09/16/2016 04:06 PM, Greg KH wrote:
>>> On Fri, Sep 16, 2016 at 03:41:09PM +0200, Jacek Anaszewski wrote:
>>>> On 09/16/2016 02:08 PM, Daniel Gorsulowski wrote:
>>>>> Hi Jacek,
>>>>>
>>>>> Am 16.09.2016 um 13:25 schrieb Jacek Anaszewski:
>>>>>> On 09/16/2016 10:15 AM, Daniel Gorsulowski wrote:
>>>>>>> Hi Jacek,
>>>>>>>
>>>>>>> Am 16.09.2016 um 09:31 schrieb Jacek Anaszewski:
>>>>>>>> Hi Daniel,
>>>>>>>>
>>>>>>>> On 09/12/2016 10:50 AM, Daniel Gorsulowski wrote:
>>>>>>>>> Hello!
>>>>>>>>>
>>>>>>>>> Please consider if I made something wrong, sending this issue. This is
>>>>>>>>> my first contact to the LKML.
>>>>>>>>> By mistake, I accessed an LED via /sys/class/leds subsystem very
>>>>>>>>> fast in
>>>>>>>>> an user application. I figured out, that the free user memory
>>>>>>>>> decreased
>>>>>>>>> constantly. So I tried to analyze the Problem and wrote a litte
>>>>>>>>> script:
>>>>>>>>>
>>>>>>>>> #!/bin/sh
>>>>>>>>> while [ 1 ]; do
>>>>>>>>>         echo 1 > /sys/class/leds/2a_service_yellow/brightness
>>>>>>>>>         echo 0 > /sys/class/leds/2a_service_yellow/brightness
>>>>>>>>> done
>>>>>>>>>
>>>>>>>>> And voila, I was able to reproduce the problem.
>>>>>>>>> So I add a bit more debugging:
>>>>>>>>>
>>>>>>>>> #!/bin/sh
>>>>>>>>> cnt=0
>>>>>>>>> while [ 1 ]; do
>>>>>>>>>         if [ `expr $cnt % 1000` -eq 0 ]; then
>>>>>>>>>                 free | grep Mem: | cut -d' ' -f25
>>>>>>>>>         fi
>>>>>>>>>         echo 1 > /sys/class/leds/2a_service_yellow/brightness
>>>>>>>>>         echo 0 > /sys/class/leds/2a_service_yellow/brightness
>>>>>>>>>         let "cnt++"
>>>>>>>>> done
>>>>>>>>>
>>>>>>>>> And huh? No memory is eaten anymore. So it looks like, the problem
>>>>>>>>> only
>>>>>>>>> occours on heavy (fast) usage of /sys/class/leds subsystem.
>>>>>>>>>
>>>>>>>>> I rewrote the script and toggled a GPIO pin, but there was no problem
>>>>>>>>> recognizable.
>>>>>>>>
>>>>>>>> I've been unable to reproduce the problem with leds-aat1290 driver
>>>>>>>> and Samsung M0 board. It must be driver specific issue.
>>>>>>>> What driver did you use?
>>>>>>>>
>>>>>>> I defined LEDS_GPIO and so I'm using leds-gpio driver.
>>>>>>> danielg@...by:~/opt/prj/ti-linux-kernel$ cat .config | grep LEDS | grep
>>>>>>> -v "^# "
>>>>>>> CONFIG_INPUT_LEDS=y
>>>>>>> CONFIG_NEW_LEDS=y
>>>>>>> CONFIG_LEDS_CLASS=y
>>>>>>> CONFIG_LEDS_GPIO=y
>>>>>>> CONFIG_LEDS_TRIGGERS=y
>>>>>>> CONFIG_LEDS_TRIGGER_TIMER=y
>>>>>>> CONFIG_LEDS_TRIGGER_ONESHOT=y
>>>>>>> CONFIG_LEDS_TRIGGER_HEARTBEAT=y
>>>>>>> CONFIG_LEDS_TRIGGER_GPIO=y
>>>>>>> CONFIG_LEDS_TRIGGER_DEFAULT_ON=y
>>>>>>> CONFIG_LEDS_TRIGGER_TRANSIENT=y
>>>>>>>
>>>>>>
>>>>>> Unfortunately I am still unable to reproduce the problem with leds-gpio.
>>>>>> I'm not observing any heavy usage with your test case:
>>>>>>
>>>>>> ~#free
>>>>>>               total       used       free     shared    buffers
>>>>>> cached
>>>>>> Mem:       1028092      61364     966728          0       8416      22396
>>>>>> -/+ buffers/cache:      30552     997540
>>>>>> Swap:            0          0          0
>>>>>>
>>>>>>
>>>>>> Actually you didn't give any numbers. What kernel version are you using?
>>>>>>
>>>>> As I wrote, the problems occurred in vanilla 4.6 kernel, but also in 4.4
>>>>> kernel (with PREEMPT-RT Patchset).
>>>>
>>>> Heh, funny coincidence. I was testing this on recent linux-leds.git,
>>>> for-next branch and was not able to detect the issue. It started to
>>>> appear after resetting HEAD to 4.8-rc2 base. Finally it turned out
>>>> that what fixes the issue is the most recent commit [1].
>>>>
>>>> Further investigation revealed that this is kobject_uevent_env(),
>>>> called from led_trigger_set(), which causes memory leaks when called
>>>> with high frequency.
>>>
>>> Really?  Where in kobject_uevent_env() is the memory leak?
>>
>> I'll chase it down when and will let you know. This may be
>> non-trivial issue as it suffices to add "sleep 0.1" between
>> brightness setting operations to prevent it.
>
> Why are you abusing uevents for flashing an LED?  Please don't do that,
> it's not what that interface is for at all.

It is called in a result of setting brightness value to LED_OFF,
which also removes registered trigger if any.

The rationale for calling kobject_uevent_env() is given in the
relevant commit message:

commit 52c47742f79d9240f90af9a6722fe8bb3fa8c0f9
Author: Colin Cross <ccross@...roid.com>
Date:   Mon Aug 27 09:31:49 2012 +0800

     leds: triggers: send uevent when changing triggers

     Some triggers create sysfs files when they are enabled. Send a uevent
     "change" notification whenever the trigger is changed to allow 
userspace
     processes such as udev to modify permissions on the new files.

     A change notification will also be sent during registration of led 
class
     devices or led triggers if the default trigger of an led class device
     is found.


-- 
Best regards,
Jacek Anaszewski