linux-kernel - Re: [PATCH 05/12] genirq/debugfs: Replace strncmp with str_has

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]

Message-ID: <708d8d79-6464-fbd3-6a62-853c29b32cc3@kernel.org>
Date:   Tue, 30 Jul 2019 12:13:22 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Chuhong Yuan <hslester96@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/12] genirq/debugfs: Replace strncmp with str_has_prefix

On 30/07/2019 11:58, Chuhong Yuan wrote:
> Thomas Gleixner <tglx@...utronix.de> 于2019年7月30日周二 下午5:17写道：
>>
>> On Mon, 29 Jul 2019, Chuhong Yuan wrote:
>>
>>> strncmp(str, const, len) is error-prone.
>>> We had better use newly introduced
>>> str_has_prefix() instead of it.
>>
>> Can you please provide a proper explanation why the below strncmp() is
>> error prone?
>>
> 
> If the size is less than 7, for example, 2, then even if buf is "tr", the
> result will still be true. This is an error.
> strncmp(str, const, len) is error-prone mainly because the len is easy
> to be wrong.
> 
>> Just running a script and copying some boiler plate changelog saying
>> 'strncmp() is error prone' does not cut it.
>>
>>> -     if (!strncmp(buf, "trigger", size)) {
>>> +     if (str_has_prefix(buf, "trigger")) {
>>
>> Especially when the resulting code is not equivalent.
>>
> 
> I think here the semantic is the comparison should only return true
> when buf is "trigger".

Not quite. It will satisfy the condition for 't', 'tr', 'trig',
'trigger', and of course 'triggerthissillyinterruptwhichImdebugging'.

I agree that the semantic is a bit bizarre and maybe not quite expected,
but still... You seem to be changing the semantic without any
justification other than "this is safer".

Thanks,

	M.
-- 
Jazz is not dead, it just smells funny...