Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] vsscanf() in lib/vsprintf.c
To:     Stefan Kanthak <kanthak@arcor.de>, linux-kernel@vger.kernel.org
References: <6C7CD73845304CDE98F6DB165904B571@H270>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
Message-ID: <91e60831-3f8b-a81d-1204-872b5ee79feb@rasmusvillemoes.dk>
Date:   Wed, 5 May 2021 16:35:52 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.1
MIME-Version: 1.0
In-Reply-To: <6C7CD73845304CDE98F6DB165904B571@H270>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 04/05/2021 21.19, Stefan Kanthak wrote:
> Hi @ll,
> 
> both <https://www.kernel.org/doc/htmldocs/kernel-api/API-sscanf.html>
> and <https://www.kernel.org/doc/htmldocs/kernel-api/API-vsscanf.html>
> are rather terse and fail to specify the supported arguments and their
> conversion specifiers/modifiers.
> 
> <https://www.kernel.org/doc/htmldocs/kernel-api/libc.html#id-1.4.3>
> tells OTOH:
> 
> | The behaviour of these functions may vary slightly from those
> | defined by ANSI, and these deviations are noted in the text.
> 
> There is but no text (see above) despite multiple deviations from
> ANSI C 
> 
> <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/plain/lib/vsprintf.c?h=v5.12>
> 
> |  /* '%*[' not yet supported, invalid format */
> ...
> |  /*
> |   * Warning: This implementation of the '[' conversion specifier
> |   * deviates from its glibc counterpart in the following ways:
> ...
> 
> More deviations (just from reading the source):
> 
> 1. no support for %p

What on earth good would that do in the kernel?

> 2. no support for conversion modifiers j and t

Could be added, but do you have a user?

> 3. no support for multibyte characters and strings, i.e. %<width>c
>    and %<width>s may split UTF-8 codepoints

So what? The kernel doesn't do a lot of text processing and wchar_t stuff.

> 4. accepts %[<width>]<modifier>[c|s], but ignores all conversion
>    modifiers

Yeah, %ls is technically accepted and treated as %s, that's mostly for
ease of parsing it seems. Do you have a use case where you'd want wchar_ts?

> 5. treats %<width><modifier>% (and combinations) as %%

What would you expect it to do? Seems to be a non-issue, gcc flags that
nonsense just fine

vs.c: In function ?v?:
vs.c:5:18: warning: conversion lacks type at end of format [-Wformat=]
    5 |  x = sscanf(s, "%l% %d", &y);
      |                  ^
vs.c:5:20: warning: unknown conversion type character ? ? in format
[-Wformat=]
    5 |  x = sscanf(s, "%l% %d", &y);
      |                    ^

> 6. accepts %<width><modifier>n (and combinations)

Again, non-issue (warning: field width used with ?%n? gnu_scanf format)

> 7. doesn't scan the input for %[...]n

? What's that supposed to mean.

> 8. uses simple_strto[u]l for the conversion modifier z, i.e. assigns
>    uint32_t to size_t, resulting in truncation

Where do you see uint32_t? The code is

                       val.u = qualifier != 'L' ?
                                simple_strtoul(str, &next, base) :
                                simple_strtoull(str, &next, base);

                case 'z':
                        *va_arg(args, size_t *) = val.u;
                        break;

so the conversion is done with simple_strtoul which return "unsigned
long". And size_t is either a typedef for "unsigned long" or "unsigned
int", so yes, of course a truncation may happen, but if the value
actually fits in a size_t, it also fits in unsigned long (as returned
from simple_strtoul) and unsigned long long (as stored in val.u).

Rasmus