linux-kernel - Re: [PATCH v2 1/6] leds: netdev trigger: use memcpy in device_name

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:   Wed, 27 Mar 2019 22:20:11 +0100
From:   Jacek Anaszewski <jacek.anaszewski@...il.com>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>,
        Pavel Machek <pavel@....cz>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, linux-leds@...r.kernel.org
Subject: Re: [PATCH v2 1/6] leds: netdev trigger: use memcpy in
 device_name_store

On 3/27/19 4:26 PM, Rasmus Villemoes wrote:
> On 26/03/2019 20.53, Jacek Anaszewski wrote:
>> Hi Rasmus,
>>
>> Thank you for the patch.
>>
>> On 3/14/19 3:06 PM, Rasmus Villemoes wrote:
>>> If userspace doesn't end the input with a newline (which can easily
>>> happen if the write happens from a C program that does write(fd,
>>> iface, strlen(iface))), we may end up including garbage from a
>>> previous, longer value in the device_name. For example
>>>
>>> # cat device_name
>>>
>>> # printf 'eth12' > device_name
>>> # cat device_name
>>> eth12
>>> # printf 'eth3' > device_name
>>> # cat device_name
>>> eth32
>>>
>>
>> Added tag:
>>
>> Fixes: 06f502f57d0d ("leds: trigger: Introduce a NETDEV trigger")
>>
>> and applied to the fixes-for-5.1-rc3 branch.
>>
> 
> You're stripping lines beginning with #. This is the commit in -next:
> 
> 
> commit 09466021a80c926aa7de68e5162bdfea2a117483
> Author: Rasmus Villemoes <linux@...musvillemoes.dk>
> Date:   Thu Mar 14 15:06:14 2019 +0100
> 
>      leds: netdev trigger: use memcpy in device_name_store
> 
>      If userspace doesn't end the input with a newline (which can easily
>      happen if the write happens from a C program that does write(fd,
>      iface, strlen(iface))), we may end up including garbage from a
>      previous, longer value in the device_name. For example
> 
>      eth12
>      eth32
> 
> which is entirely useless and confusing. Please fix this before it
> actually hits mainline. And you may want to look into "git commit
> --cleanup" and/or "git config commit.cleanup" (scissors is much better
> than the default strip).

Thanks for the heads-up. I must admit I'm hitting into that for the
first time. After "git am" it was all OK, but it got screwed up after
"git rebase -i". And having "commit.cleanup = scissors" set globally all
the time is annoying if one extensively uses interactive rebase for
rewording commit messages. It entails the need for manual removal of
the whole stuff that appears then after actual commit message prepended
with "#" comment characters.

This is probably the reason why people use often other characters
for command prompt (see the other fix for ledtrig-netdev). Note that, 
while it is of no so great importance for mainline, when people cherry
pick such patch to their out-of-tree repositories and then rebase, they
hit into the same issue.

-- 
Best regards,
Jacek Anaszewski