linux-kernel - Re: [PATCH] kstrtox: make kstrtobool_from

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: <20180213174535.GA28871@avx2>
Date:   Tue, 13 Feb 2018 20:45:35 +0300
From:   Alexey Dobriyan <adobriyan@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] kstrtox: make kstrtobool_from_user() very strict

On Tue, Feb 13, 2018 at 07:18:08PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 13, 2018 at 7:11 PM, Alexey Dobriyan <adobriyan@...il.com> wrote:
> > On Tue, Feb 13, 2018 at 06:02:27PM +0200, Andy Shevchenko wrote:
> >> On Sat, Feb 10, 2018 at 11:11 PM, Alexey Dobriyan <adobriyan@...il.com> wrote:
> >> > Once upon a time module parameter parsing code accepted
> >> > 0, 1, y, n, Y and N for boolean values. Gratituous but contained
> >> > to module code and thus tolerable.
> >> >
> >> > Commit ef951599074ba4fad2d0efa0a977129b41e6d203
> >> > ("lib: move strtobool() to kstrtobool()") promoted that ugly wart
> >> > to kstrtobool() and, more importantly, kstrtobool_from_user().
> >> >
> >> > Later set of accepted values was expanded to "on" and "of".
> >> > Now there are 6+8=14(!) valid strings for a boolean.
> >> >
> >> > This patch reduces set of accepted values to "0" and "1"
> >> > (with optional newline) in spirit with other kstrto*() functions.
> >> >
> >> > I'm starting with kstrtobool_from_user() as it is explicitly designed
> >> > to be used for interacting with userspace. Currently there are 9 users
> >> > all debug code, so there is hope.
> >> >
> >> > Please send before 4.16 so no real users start to depend on verbose behaviour.
> 
> >> NACK.
> >> You basically are breaking ABI here. I don't see a zillion patches
> >> which adds a tons of duplicate code to the corresponding users.
> >
> > Please do "find . -type f -name '*.[ch]' | xargs grep kstrtobool_from_user -w'
> > before talking about zillion of patches.
> 
> If you wonder, I did it of course. The stylistic device I used here is
> called "hyperbole".
> 
> Even for that dozen or so users I don't see how you handled the change.

All the users of kstrtobool_from_user() are in debug code, so there are
no users.

I'd flip kstrtobool() as well, but you're guys are taking hostages now:
we sneaked in dubious change (the author of a file was not on Cc list,
but 11 other people were) and we won't let you back it out because ABI
and users.

Plan for kstrtobool() is to revert those conversions for users which
used "on" and "off" and param parsing code so that new users can use
0 and 1 consistently across the kernel.