linux-kernel - Re: [PATCH] led/ledtrig-heartbeat: Convert timers to use

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: <CAGXu5jKFhz6KEFB0QDkrrhCBO3eRF8xAAq+MP_ZBHDxZk6QN8A@mail.gmail.com>
Date:   Fri, 6 Oct 2017 13:21:50 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Jacek Anaszewski <jacek.anaszewski@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Richard Purdie <rpurdie@...ys.net>,
        Pavel Machek <pavel@....cz>, Zhang Bo <bo.zhang@....com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Ingo Molnar <mingo@...nel.org>,
        Linux LED Subsystem <linux-leds@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] led/ledtrig-heartbeat: Convert timers to use

On Fri, Oct 6, 2017 at 1:18 PM, Jacek Anaszewski
<jacek.anaszewski@...il.com> wrote:
> Hi Kees,
>
> Thanks for the patch.
>
> On 10/05/2017 02:49 AM, Kees Cook wrote:
>> Instead of using .data directly, convert to from_timer. Since the
>> trigger_data is allocated separately, the led_cdev must be explicitly
>> tracked for the callback.
>>
>> Cc: Richard Purdie <rpurdie@...ys.net>
>> Cc: Jacek Anaszewski <jacek.anaszewski@...il.com>
>> Cc: Pavel Machek <pavel@....cz>
>> Cc: Zhang Bo <bo.zhang@....com>
>> Cc: Linus Walleij <linus.walleij@...aro.org>
>> Cc: Ingo Molnar <mingo@...nel.org>
>> Cc: linux-leds@...r.kernel.org
>> Cc: Thomas Gleixner <tglx@...utronix.de>
>> Signed-off-by: Kees Cook <keescook@...omium.org>
>> ---
>> This requires commit 686fef928bba ("timer: Prepare to change timer
>> callback argument type") in v4.14-rc3, but should be otherwise
>> stand-alone.
>> ---
>>  drivers/leds/trigger/ledtrig-heartbeat.c | 16 ++++++++++------
>>  1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/leds/trigger/ledtrig-heartbeat.c b/drivers/leds/trigger/ledtrig-heartbeat.c
>> index e95ea65380c8..f0896de410b8 100644
>> --- a/drivers/leds/trigger/ledtrig-heartbeat.c
>> +++ b/drivers/leds/trigger/ledtrig-heartbeat.c
>> @@ -25,19 +25,23 @@
>>  static int panic_heartbeats;
>>
>>  struct heartbeat_trig_data {
>> +     struct led_classdev *led_cdev;
>>       unsigned int phase;
>>       unsigned int period;
>>       struct timer_list timer;
>>       unsigned int invert;
>>  };
>>
>> -static void led_heartbeat_function(unsigned long data)
>> +static void led_heartbeat_function(struct timer_list *t)
>>  {
>> -     struct led_classdev *led_cdev = (struct led_classdev *) data;
>> -     struct heartbeat_trig_data *heartbeat_data = led_cdev->trigger_data;
>> +     struct heartbeat_trig_data *heartbeat_data =
>> +             from_timer(heartbeat_data, t, timer);
>> +     struct led_classdev *led_cdev;
>>       unsigned long brightness = LED_OFF;
>>       unsigned long delay = 0;
>>
>> +     led_cdev = heartbeat_data->led_cdev;
>> +
>>       if (unlikely(panic_heartbeats)) {
>>               led_set_brightness_nosleep(led_cdev, LED_OFF);
>>               return;
>> @@ -127,18 +131,18 @@ static void heartbeat_trig_activate(struct led_classdev *led_cdev)
>>               return;
>>
>>       led_cdev->trigger_data = heartbeat_data;
>> +     heartbeat_data->led_cdev = led_cdev;
>>       rc = device_create_file(led_cdev->dev, &dev_attr_invert);
>>       if (rc) {
>>               kfree(led_cdev->trigger_data);
>>               return;
>>       }
>>
>> -     setup_timer(&heartbeat_data->timer,
>> -                 led_heartbeat_function, (unsigned long) led_cdev);
>> +     timer_setup(&heartbeat_data->timer, led_heartbeat_function, 0);
>>       heartbeat_data->phase = 0;
>>       if (!led_cdev->blink_brightness)
>>               led_cdev->blink_brightness = led_cdev->max_brightness;
>> -     led_heartbeat_function(heartbeat_data->timer.data);
>> +     led_heartbeat_function(&heartbeat_data->timer);
>>       set_bit(LED_BLINK_SW, &led_cdev->work_flags);
>>       led_cdev->activated = true;
>>  }
>>
>
> It seems that the patch title is somewhat incomplete, so I amended
> it as follows:
>
> s/to use/to use timer_setup()/

Yeah, thanks for fixing that. This got pointed out elsewhere and I've
since hunted down and fixed my scripts to not accidentally truncate
subjects. *sigh* :)

> and applied to the for-next branch of linux-leds.git after rebasing
> it onto 4.14-rc3.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security