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] [day] [month] [year] [list]

Message-ID: <CANhBUQ0uWu5yt3jXWxgH1a-i2vuHpkxwPbQ4PZdpPoi+LJ+P1Q@mail.gmail.com>
Date:   Tue, 30 Jul 2019 19:20:47 +0800
From:   Chuhong Yuan <hslester96@...il.com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/12] genirq/debugfs: Replace strncmp with str_has_prefix

Marc Zyngier <maz@...nel.org> 于2019年7月30日周二 下午7:13写道：
>
> 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".
>

I am sorry about that... It is my fault.
I will improve my script and avoid such mistakes.
Thanks for your correction.

Regards,
Chuhong

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