linux-kernel - Re: [PATCH v4 2/8] lib: add "on" and "off" to strtobool

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: <CAGXu5jLXuz529eyE9o0u88+iBzUqbB_Mh3Jbp43EZNgJDnWu2Q@mail.gmail.com>
Date:	Fri, 22 Jan 2016 15:29:45 -0800
From:	Kees Cook <keescook@...omium.org>
To:	Joe Perches <joe@...ches.com>
Cc:	Ingo Molnar <mingo@...hat.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Daniel Borkmann <daniel@...earbox.net>,
	Andy Lutomirski <luto@...capital.net>,
	"H. Peter Anvin" <hpa@...or.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Mathias Krause <minipli@...glemail.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	"x86@...nel.org" <x86@...nel.org>, Arnd Bergmann <arnd@...db.de>,
	PaX Team <pageexec@...email.hu>,
	Emese Revfy <re.emese@...il.com>,
	"kernel-hardening@...ts.openwall.com" 
	<kernel-hardening@...ts.openwall.com>,
	LKML <linux-kernel@...r.kernel.org>,
	linux-arch <linux-arch@...r.kernel.org>
Subject: Re: [PATCH v4 2/8] lib: add "on" and "off" to strtobool

On Tue, Jan 19, 2016 at 6:09 PM, Joe Perches <joe@...ches.com> wrote:
> On Tue, 2016-01-19 at 10:08 -0800, Kees Cook wrote:
>> Several places in the kernel expect to use "on" and "off" for their
>> boolean signifiers, so add them to strtobool.
>
> Several places in the kernel use a char address like
> fs/cifs/cifs_debug.c
>
>
>         char c;
>         ...
>
>
>         if (strtobool(&c, ...))
>
> Using s[1] might cause problems for those uses.

Oh ew. Thanks for noticing that.

>> diff --git a/lib/string.c b/lib/string.c
> []
>> @@ -635,12 +635,15 @@ EXPORT_SYMBOL(sysfs_streq);
>>   * @s: input string
>>   * @res: result
>>   *
>> - * This routine returns 0 iff the first character is one of 'Yy1Nn0'.
>> - * Otherwise it will return -EINVAL.  Value pointed to by res is
>> - * updated upon finding a match.
>> + * This routine returns 0 iff the first character is one of 'Yy1Nn0', or
>> + * [oO][NnFf] for "on" and "off". Otherwise it will return -EINVAL.  Value
>> + * pointed to by res is updated upon finding a match.
>>   */
>>  int strtobool(const char *s, bool *res)
>>  {
>> +     if (!s)
>> +             return -EINVAL;
>> +
>>       switch (s[0]) {
>>       case 'y':
>>       case 'Y':
>> @@ -652,6 +655,21 @@ int strtobool(const char *s, bool *res)
>>       case '0':
>>               *res = false;
>>               break;
>> +     case 'o':
>> +     case 'O':
>> +             switch (s[1]) {
>> +             case 'n':
>> +             case 'N':
>> +                     *res = true;
>> +                     break;
>> +             case 'f':
>> +             case 'F':
>
> Perhaps
>                 switch (tolower(s[1])) {
> is more readable

I opted to let the compiler deal with optimizing this, and I left the
switch statement as close to original as possible.

-Kees

>
>> +                     *res = false;
>> +                     break;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +             break;
>
> or maybe /* fallthrough */
>
>>       default:
>>               return -EINVAL;
>>       }
>



-- 
Kees Cook
Chrome OS & Brillo Security