linux-kernel - Re: [PATCH RFC] auxdisplay: charlcd: Fix and clean up handling of x/y commands

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:   Tue, 27 Feb 2018 21:56:53 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Willy Tarreau <w@....eu>
Cc:     Geert Uytterhoeven <geert@...ux-m68k.org>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Robert Abel <rabel@...ertabel.eu>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] auxdisplay: charlcd: Fix and clean up handling of x/y commands

On Tue, Feb 27, 2018 at 9:13 PM, Willy Tarreau <w@....eu> wrote:
> On Tue, Feb 27, 2018 at 08:32:21PM +0100, Miguel Ojeda wrote:
>> The current version is not parsing multiple x/y commands as the code
>> originally intended. On top of that, kstrtoul() expects
>> NULL-terminated strings. Finally, the code had to do two passes over
>> the string, while now only one is done.
>>
>> Some explanations about the supported syntax are added as well.
>
> Thanks Miguel for doing this. To be fair, I personally think that the code

You're welcome!

> was significantly complexified compared to the original version, just for
> the purpose of using one library function designed to multiply by ten and
> add a number. Not to mention that it now requires a copy before parsing.

I agree. I wrote this version like this so that parse_n() can be
removed in the future if we have access to an in-place alternative to
the kstrtoul*() functions (cf. the discussion in the other thread with
Andy); which would take the copy away as well.

> For me it simply means that the initial code change was a wrong idea in
> the end. But at least with your version, the bugs introduced by the
> previous "fix" should now be gone, which is a good thing.

Thanks! :-) This one has the "advantage" that it only goes by the
string once (i.e. no initial strchr()) and that gotoxy() is not called
unless the full command is valid (but I am not sure whether this is OK
or not for the HW, though).

Cheers,
Miguel

>
> Thanks,
> Willy