* [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
@ 2023-06-10 2:57 ` Demi Marie Obenour
2023-06-10 7:51 ` Christoph Hellwig
2023-06-10 12:59 ` kernel test robot
2023-06-10 2:57 ` [PATCH 3/4] Add strict version of vsscanf() Demi Marie Obenour
` (5 subsequent siblings)
6 siblings, 2 replies; 13+ messages in thread
From: Demi Marie Obenour @ 2023-06-10 2:57 UTC (permalink / raw)
To: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes
Cc: Demi Marie Obenour, linux-doc, linux-kernel, xen-devel
Userspace sets errno to ERANGE, but the kernel can't do that.
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
include/linux/limits.h | 1 +
include/linux/mfd/wl1273-core.h | 3 --
include/vdso/limits.h | 3 ++
lib/vsprintf.c | 80 ++++++++++++++++++++++++---------
4 files changed, 63 insertions(+), 24 deletions(-)
diff --git a/include/linux/limits.h b/include/linux/limits.h
index f6bcc936901071f496e3e85bb6e1d93905b12e32..8f7fd85b41fb46e6992d9e5912da00424119227a 100644
--- a/include/linux/limits.h
+++ b/include/linux/limits.h
@@ -8,6 +8,7 @@
#define SIZE_MAX (~(size_t)0)
#define SSIZE_MAX ((ssize_t)(SIZE_MAX >> 1))
+#define SSIZE_MIN (-SSIZE_MAX - 1)
#define PHYS_ADDR_MAX (~(phys_addr_t)0)
#define U8_MAX ((u8)~0U)
diff --git a/include/linux/mfd/wl1273-core.h b/include/linux/mfd/wl1273-core.h
index c28cf76d5c31ee1c94a9319a2e2d318bf00283a6..b81a229135ed9f756c749122a8341816031c8311 100644
--- a/include/linux/mfd/wl1273-core.h
+++ b/include/linux/mfd/wl1273-core.h
@@ -204,9 +204,6 @@
WL1273_IS2_TRI_OPT | \
WL1273_IS2_RATE_48K)
-#define SCHAR_MIN (-128)
-#define SCHAR_MAX 127
-
#define WL1273_FR_EVENT BIT(0)
#define WL1273_BL_EVENT BIT(1)
#define WL1273_RDS_EVENT BIT(2)
diff --git a/include/vdso/limits.h b/include/vdso/limits.h
index 0197888ad0e00b2f853d3f25ffa764f61cca7385..0cad0a2490e5efc194d874025eb3e3b846a5c7b4 100644
--- a/include/vdso/limits.h
+++ b/include/vdso/limits.h
@@ -2,6 +2,9 @@
#ifndef __VDSO_LIMITS_H
#define __VDSO_LIMITS_H
+#define UCHAR_MAX ((unsigned char)~0U)
+#define SCHAR_MAX ((signed char)(UCHAR_MAX >> 1))
+#define SCHAR_MIN ((signed char)(-SCHAR_MAX - 1))
#define USHRT_MAX ((unsigned short)~0U)
#define SHRT_MAX ((short)(USHRT_MAX >> 1))
#define SHRT_MIN ((short)(-SHRT_MAX - 1))
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index a60d348efb276d66ca07fe464883408df7fdab97..9846d2385f5b9e8f3945a5664d81047e97cf10d5 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -59,7 +59,7 @@
bool no_hash_pointers __ro_after_init;
EXPORT_SYMBOL_GPL(no_hash_pointers);
-static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base)
+static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base, bool *overflow)
{
const char *cp;
unsigned long long result = 0ULL;
@@ -71,6 +71,8 @@ static noinline unsigned long long simple_strntoull(const char *startp, size_t m
if (prefix_chars < max_chars) {
rv = _parse_integer_limit(cp, base, &result, max_chars - prefix_chars);
/* FIXME */
+ if (overflow)
+ *overflow = !!(rv & KSTRTOX_OVERFLOW);
cp += (rv & ~KSTRTOX_OVERFLOW);
} else {
/* Field too short for prefix + digit, skip over without converting */
@@ -94,7 +96,7 @@ static noinline unsigned long long simple_strntoull(const char *startp, size_t m
noinline
unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
{
- return simple_strntoull(cp, INT_MAX, endp, base);
+ return simple_strntoull(cp, INT_MAX, endp, base, NULL);
}
EXPORT_SYMBOL(simple_strtoull);
@@ -130,18 +132,22 @@ long simple_strtol(const char *cp, char **endp, unsigned int base)
EXPORT_SYMBOL(simple_strtol);
static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
- unsigned int base)
+ unsigned int base, bool *overflow)
{
+ unsigned long long minand;
+ bool negate;
+
/*
* simple_strntoull() safely handles receiving max_chars==0 in the
* case cp[0] == '-' && max_chars == 1.
* If max_chars == 0 we can drop through and pass it to simple_strntoull()
* and the content of *cp is irrelevant.
*/
- if (*cp == '-' && max_chars > 0)
- return -simple_strntoull(cp + 1, max_chars - 1, endp, base);
-
- return simple_strntoull(cp, max_chars, endp, base);
+ negate = *cp == '-' && max_chars > 0;
+ minand = simple_strntoull(cp + negate, max_chars - negate, endp, base, overflow);
+ if (minand > (unsigned long long)LONG_MAX + negate)
+ *overflow = true;
+ return negate ? -minand : minand;
}
static noinline_for_stack
@@ -3427,7 +3433,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
unsigned long long u;
} val;
s16 field_width;
- bool is_sign;
+ bool is_sign, overflow;
while (*fmt) {
/* skip any white space in format */
@@ -3635,45 +3641,77 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
if (is_sign)
val.s = simple_strntoll(str,
field_width >= 0 ? field_width : INT_MAX,
- &next, base);
+ &next, base, &overflow);
else
val.u = simple_strntoull(str,
field_width >= 0 ? field_width : INT_MAX,
- &next, base);
+ &next, base, &overflow);
+ if (unlikely(overflow))
+ return -ERANGE;
switch (qualifier) {
case 'H': /* that's 'hh' in format */
- if (is_sign)
+ if (is_sign) {
+ if (unlikely(val.s < SCHAR_MIN || val.s > SCHAR_MAX))
+ return -ERANGE;
*va_arg(args, signed char *) = val.s;
- else
+ } else {
+ if (unlikely(val.u > UCHAR_MAX))
+ return -ERANGE;
*va_arg(args, unsigned char *) = val.u;
+ }
break;
case 'h':
- if (is_sign)
+ if (is_sign) {
+ if (unlikely(val.s < SHRT_MIN || val.s > SHRT_MAX))
+ return -ERANGE;
*va_arg(args, short *) = val.s;
- else
+ } else {
+ if (unlikely(val.u > USHRT_MAX))
+ return -ERANGE;
*va_arg(args, unsigned short *) = val.u;
+ }
break;
case 'l':
- if (is_sign)
+ if (is_sign) {
+ if (unlikely(val.s < LONG_MIN || val.s > LONG_MAX))
+ return -ERANGE;
*va_arg(args, long *) = val.s;
- else
+ } else {
+ if (unlikely(val.u > ULONG_MAX))
+ return -ERANGE;
*va_arg(args, unsigned long *) = val.u;
+ }
break;
case 'L':
- if (is_sign)
+ /* No overflow check needed */
+ if (is_sign) {
*va_arg(args, long long *) = val.s;
- else
+ } else {
*va_arg(args, unsigned long long *) = val.u;
+ }
break;
case 'z':
- *va_arg(args, size_t *) = val.u;
+ if (is_sign) {
+ if (unlikely(val.s < SSIZE_MIN || val.s > SSIZE_MAX))
+ return -ERANGE;
+ *va_arg(args, ssize_t *) = val.s;
+ } else {
+ if (unlikely(val.u > SIZE_MAX))
+ return -ERANGE;
+ *va_arg(args, size_t *) = val.u;
+ }
break;
default:
- if (is_sign)
+ if (is_sign) {
+ if (unlikely(val.s < INT_MIN || val.s > INT_MAX))
+ return -ERANGE;
*va_arg(args, int *) = val.s;
- else
+ } else {
+ if (unlikely(val.u > UINT_MAX))
+ return -ERANGE;
*va_arg(args, unsigned int *) = val.u;
+ }
break;
}
num++;
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow
2023-06-10 2:57 ` [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow Demi Marie Obenour
@ 2023-06-10 7:51 ` Christoph Hellwig
2023-06-10 8:10 ` Richard Weinberger
2023-06-10 12:59 ` kernel test robot
1 sibling, 1 reply; 13+ messages in thread
From: Christoph Hellwig @ 2023-06-10 7:51 UTC (permalink / raw)
To: Demi Marie Obenour, Richard Weinberger, Linus Torvalds
Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc,
linux-kernel, xen-devel
[Adding Richard and Linus as they're having another overflow checking
discussion and we should probably merge those]
On Fri, Jun 09, 2023 at 10:57:57PM -0400, Demi Marie Obenour wrote:
> Userspace sets errno to ERANGE, but the kernel can't do that.
That seems like a very parse commit log, and also kinda besides
the point - the kernel always returns error in-line and not through
errno. I think you need to document here why we want to do the
overflow checking (not that I doubt it, but it really needs to be
in the commit message).
Leaving the rest of the quote here for the new arrivals.
>
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> include/linux/limits.h | 1 +
> include/linux/mfd/wl1273-core.h | 3 --
> include/vdso/limits.h | 3 ++
> lib/vsprintf.c | 80 ++++++++++++++++++++++++---------
> 4 files changed, 63 insertions(+), 24 deletions(-)
>
> diff --git a/include/linux/limits.h b/include/linux/limits.h
> index f6bcc936901071f496e3e85bb6e1d93905b12e32..8f7fd85b41fb46e6992d9e5912da00424119227a 100644
> --- a/include/linux/limits.h
> +++ b/include/linux/limits.h
> @@ -8,6 +8,7 @@
>
> #define SIZE_MAX (~(size_t)0)
> #define SSIZE_MAX ((ssize_t)(SIZE_MAX >> 1))
> +#define SSIZE_MIN (-SSIZE_MAX - 1)
> #define PHYS_ADDR_MAX (~(phys_addr_t)0)
>
> #define U8_MAX ((u8)~0U)
> diff --git a/include/linux/mfd/wl1273-core.h b/include/linux/mfd/wl1273-core.h
> index c28cf76d5c31ee1c94a9319a2e2d318bf00283a6..b81a229135ed9f756c749122a8341816031c8311 100644
> --- a/include/linux/mfd/wl1273-core.h
> +++ b/include/linux/mfd/wl1273-core.h
> @@ -204,9 +204,6 @@
> WL1273_IS2_TRI_OPT | \
> WL1273_IS2_RATE_48K)
>
> -#define SCHAR_MIN (-128)
> -#define SCHAR_MAX 127
> -
> #define WL1273_FR_EVENT BIT(0)
> #define WL1273_BL_EVENT BIT(1)
> #define WL1273_RDS_EVENT BIT(2)
> diff --git a/include/vdso/limits.h b/include/vdso/limits.h
> index 0197888ad0e00b2f853d3f25ffa764f61cca7385..0cad0a2490e5efc194d874025eb3e3b846a5c7b4 100644
> --- a/include/vdso/limits.h
> +++ b/include/vdso/limits.h
> @@ -2,6 +2,9 @@
> #ifndef __VDSO_LIMITS_H
> #define __VDSO_LIMITS_H
>
> +#define UCHAR_MAX ((unsigned char)~0U)
> +#define SCHAR_MAX ((signed char)(UCHAR_MAX >> 1))
> +#define SCHAR_MIN ((signed char)(-SCHAR_MAX - 1))
> #define USHRT_MAX ((unsigned short)~0U)
> #define SHRT_MAX ((short)(USHRT_MAX >> 1))
> #define SHRT_MIN ((short)(-SHRT_MAX - 1))
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index a60d348efb276d66ca07fe464883408df7fdab97..9846d2385f5b9e8f3945a5664d81047e97cf10d5 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -59,7 +59,7 @@
> bool no_hash_pointers __ro_after_init;
> EXPORT_SYMBOL_GPL(no_hash_pointers);
>
> -static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base)
> +static noinline unsigned long long simple_strntoull(const char *startp, size_t max_chars, char **endp, unsigned int base, bool *overflow)
> {
> const char *cp;
> unsigned long long result = 0ULL;
> @@ -71,6 +71,8 @@ static noinline unsigned long long simple_strntoull(const char *startp, size_t m
> if (prefix_chars < max_chars) {
> rv = _parse_integer_limit(cp, base, &result, max_chars - prefix_chars);
> /* FIXME */
> + if (overflow)
> + *overflow = !!(rv & KSTRTOX_OVERFLOW);
> cp += (rv & ~KSTRTOX_OVERFLOW);
> } else {
> /* Field too short for prefix + digit, skip over without converting */
> @@ -94,7 +96,7 @@ static noinline unsigned long long simple_strntoull(const char *startp, size_t m
> noinline
> unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int base)
> {
> - return simple_strntoull(cp, INT_MAX, endp, base);
> + return simple_strntoull(cp, INT_MAX, endp, base, NULL);
> }
> EXPORT_SYMBOL(simple_strtoull);
>
> @@ -130,18 +132,22 @@ long simple_strtol(const char *cp, char **endp, unsigned int base)
> EXPORT_SYMBOL(simple_strtol);
>
> static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
> - unsigned int base)
> + unsigned int base, bool *overflow)
> {
> + unsigned long long minand;
> + bool negate;
> +
> /*
> * simple_strntoull() safely handles receiving max_chars==0 in the
> * case cp[0] == '-' && max_chars == 1.
> * If max_chars == 0 we can drop through and pass it to simple_strntoull()
> * and the content of *cp is irrelevant.
> */
> - if (*cp == '-' && max_chars > 0)
> - return -simple_strntoull(cp + 1, max_chars - 1, endp, base);
> -
> - return simple_strntoull(cp, max_chars, endp, base);
> + negate = *cp == '-' && max_chars > 0;
> + minand = simple_strntoull(cp + negate, max_chars - negate, endp, base, overflow);
> + if (minand > (unsigned long long)LONG_MAX + negate)
> + *overflow = true;
> + return negate ? -minand : minand;
> }
>
> static noinline_for_stack
> @@ -3427,7 +3433,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
> unsigned long long u;
> } val;
> s16 field_width;
> - bool is_sign;
> + bool is_sign, overflow;
>
> while (*fmt) {
> /* skip any white space in format */
> @@ -3635,45 +3641,77 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
> if (is_sign)
> val.s = simple_strntoll(str,
> field_width >= 0 ? field_width : INT_MAX,
> - &next, base);
> + &next, base, &overflow);
> else
> val.u = simple_strntoull(str,
> field_width >= 0 ? field_width : INT_MAX,
> - &next, base);
> + &next, base, &overflow);
> + if (unlikely(overflow))
> + return -ERANGE;
>
> switch (qualifier) {
> case 'H': /* that's 'hh' in format */
> - if (is_sign)
> + if (is_sign) {
> + if (unlikely(val.s < SCHAR_MIN || val.s > SCHAR_MAX))
> + return -ERANGE;
> *va_arg(args, signed char *) = val.s;
> - else
> + } else {
> + if (unlikely(val.u > UCHAR_MAX))
> + return -ERANGE;
> *va_arg(args, unsigned char *) = val.u;
> + }
> break;
> case 'h':
> - if (is_sign)
> + if (is_sign) {
> + if (unlikely(val.s < SHRT_MIN || val.s > SHRT_MAX))
> + return -ERANGE;
> *va_arg(args, short *) = val.s;
> - else
> + } else {
> + if (unlikely(val.u > USHRT_MAX))
> + return -ERANGE;
> *va_arg(args, unsigned short *) = val.u;
> + }
> break;
> case 'l':
> - if (is_sign)
> + if (is_sign) {
> + if (unlikely(val.s < LONG_MIN || val.s > LONG_MAX))
> + return -ERANGE;
> *va_arg(args, long *) = val.s;
> - else
> + } else {
> + if (unlikely(val.u > ULONG_MAX))
> + return -ERANGE;
> *va_arg(args, unsigned long *) = val.u;
> + }
> break;
> case 'L':
> - if (is_sign)
> + /* No overflow check needed */
> + if (is_sign) {
> *va_arg(args, long long *) = val.s;
> - else
> + } else {
> *va_arg(args, unsigned long long *) = val.u;
> + }
> break;
> case 'z':
> - *va_arg(args, size_t *) = val.u;
> + if (is_sign) {
> + if (unlikely(val.s < SSIZE_MIN || val.s > SSIZE_MAX))
> + return -ERANGE;
> + *va_arg(args, ssize_t *) = val.s;
> + } else {
> + if (unlikely(val.u > SIZE_MAX))
> + return -ERANGE;
> + *va_arg(args, size_t *) = val.u;
> + }
> break;
> default:
> - if (is_sign)
> + if (is_sign) {
> + if (unlikely(val.s < INT_MIN || val.s > INT_MAX))
> + return -ERANGE;
> *va_arg(args, int *) = val.s;
> - else
> + } else {
> + if (unlikely(val.u > UINT_MAX))
> + return -ERANGE;
> *va_arg(args, unsigned int *) = val.u;
> + }
> break;
> }
> num++;
> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab
>
---end quoted text---
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow
2023-06-10 7:51 ` Christoph Hellwig
@ 2023-06-10 8:10 ` Richard Weinberger
0 siblings, 0 replies; 13+ messages in thread
From: Richard Weinberger @ 2023-06-10 8:10 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Demi Marie Obenour, torvalds, Dwaipayan Ray, Lukas Bulwahn,
Joe Perches, Jonathan Corbet, Federico Vaga, jgross,
Stefano Stabellini, Oleksandr Tyshchenko, Lee Jones,
Andy Lutomirski, tglx, Vincenzo Frascino, Petr Mladek,
Steven Rostedt, senozhatsky, Andy Shevchenko, Rasmus Villemoes,
Linux Doc Mailing List, linux-kernel, xen-devel
----- Ursprüngliche Mail -----
> Von: "Christoph Hellwig" <hch@infradead.org>
> [Adding Richard and Linus as they're having another overflow checking
> discussion and we should probably merge those]
Thx for letting me know!
> On Fri, Jun 09, 2023 at 10:57:57PM -0400, Demi Marie Obenour wrote:
>> Userspace sets errno to ERANGE, but the kernel can't do that.
>
> That seems like a very parse commit log, and also kinda besides
> the point - the kernel always returns error in-line and not through
> errno. I think you need to document here why we want to do the
> overflow checking (not that I doubt it, but it really needs to be
> in the commit message).
>
> Leaving the rest of the quote here for the new arrivals.
>
>>
>> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
>> ---
>> include/linux/limits.h | 1 +
>> include/linux/mfd/wl1273-core.h | 3 --
>> include/vdso/limits.h | 3 ++
>> lib/vsprintf.c | 80 ++++++++++++++++++++++++---------
>> 4 files changed, 63 insertions(+), 24 deletions(-)
>>
>> diff --git a/include/linux/limits.h b/include/linux/limits.h
>> index
>> f6bcc936901071f496e3e85bb6e1d93905b12e32..8f7fd85b41fb46e6992d9e5912da00424119227a
>> 100644
>> --- a/include/linux/limits.h
>> +++ b/include/linux/limits.h
>> @@ -8,6 +8,7 @@
>>
>> #define SIZE_MAX (~(size_t)0)
>> #define SSIZE_MAX ((ssize_t)(SIZE_MAX >> 1))
>> +#define SSIZE_MIN (-SSIZE_MAX - 1)
>> #define PHYS_ADDR_MAX (~(phys_addr_t)0)
>>
>> #define U8_MAX ((u8)~0U)
>> diff --git a/include/linux/mfd/wl1273-core.h b/include/linux/mfd/wl1273-core.h
>> index
>> c28cf76d5c31ee1c94a9319a2e2d318bf00283a6..b81a229135ed9f756c749122a8341816031c8311
>> 100644
>> --- a/include/linux/mfd/wl1273-core.h
>> +++ b/include/linux/mfd/wl1273-core.h
>> @@ -204,9 +204,6 @@
>> WL1273_IS2_TRI_OPT | \
>> WL1273_IS2_RATE_48K)
>>
>> -#define SCHAR_MIN (-128)
>> -#define SCHAR_MAX 127
>> -
>> #define WL1273_FR_EVENT BIT(0)
>> #define WL1273_BL_EVENT BIT(1)
>> #define WL1273_RDS_EVENT BIT(2)
>> diff --git a/include/vdso/limits.h b/include/vdso/limits.h
>> index
>> 0197888ad0e00b2f853d3f25ffa764f61cca7385..0cad0a2490e5efc194d874025eb3e3b846a5c7b4
>> 100644
>> --- a/include/vdso/limits.h
>> +++ b/include/vdso/limits.h
>> @@ -2,6 +2,9 @@
>> #ifndef __VDSO_LIMITS_H
>> #define __VDSO_LIMITS_H
>>
>> +#define UCHAR_MAX ((unsigned char)~0U)
>> +#define SCHAR_MAX ((signed char)(UCHAR_MAX >> 1))
>> +#define SCHAR_MIN ((signed char)(-SCHAR_MAX - 1))
>> #define USHRT_MAX ((unsigned short)~0U)
>> #define SHRT_MAX ((short)(USHRT_MAX >> 1))
>> #define SHRT_MIN ((short)(-SHRT_MAX - 1))
>> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
>> index
>> a60d348efb276d66ca07fe464883408df7fdab97..9846d2385f5b9e8f3945a5664d81047e97cf10d5
>> 100644
>> --- a/lib/vsprintf.c
>> +++ b/lib/vsprintf.c
>> @@ -59,7 +59,7 @@
>> bool no_hash_pointers __ro_after_init;
>> EXPORT_SYMBOL_GPL(no_hash_pointers);
>>
>> -static noinline unsigned long long simple_strntoull(const char *startp, size_t
>> max_chars, char **endp, unsigned int base)
>> +static noinline unsigned long long simple_strntoull(const char *startp, size_t
>> max_chars, char **endp, unsigned int base, bool *overflow)
>> {
>> const char *cp;
>> unsigned long long result = 0ULL;
>> @@ -71,6 +71,8 @@ static noinline unsigned long long simple_strntoull(const char
>> *startp, size_t m
>> if (prefix_chars < max_chars) {
>> rv = _parse_integer_limit(cp, base, &result, max_chars - prefix_chars);
>> /* FIXME */
>> + if (overflow)
>> + *overflow = !!(rv & KSTRTOX_OVERFLOW);
>> cp += (rv & ~KSTRTOX_OVERFLOW);
>> } else {
>> /* Field too short for prefix + digit, skip over without converting */
>> @@ -94,7 +96,7 @@ static noinline unsigned long long simple_strntoull(const char
>> *startp, size_t m
>> noinline
>> unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int
>> base)
>> {
>> - return simple_strntoull(cp, INT_MAX, endp, base);
>> + return simple_strntoull(cp, INT_MAX, endp, base, NULL);
>> }
>> EXPORT_SYMBOL(simple_strtoull);
>>
>> @@ -130,18 +132,22 @@ long simple_strtol(const char *cp, char **endp, unsigned
>> int base)
>> EXPORT_SYMBOL(simple_strtol);
>>
>> static long long simple_strntoll(const char *cp, size_t max_chars, char **endp,
>> - unsigned int base)
>> + unsigned int base, bool *overflow)
>> {
>> + unsigned long long minand;
>> + bool negate;
>> +
>> /*
>> * simple_strntoull() safely handles receiving max_chars==0 in the
>> * case cp[0] == '-' && max_chars == 1.
>> * If max_chars == 0 we can drop through and pass it to simple_strntoull()
>> * and the content of *cp is irrelevant.
>> */
>> - if (*cp == '-' && max_chars > 0)
>> - return -simple_strntoull(cp + 1, max_chars - 1, endp, base);
>> -
>> - return simple_strntoull(cp, max_chars, endp, base);
>> + negate = *cp == '-' && max_chars > 0;
>> + minand = simple_strntoull(cp + negate, max_chars - negate, endp, base,
>> overflow);
>> + if (minand > (unsigned long long)LONG_MAX + negate)
>> + *overflow = true;
>> + return negate ? -minand : minand;
>> }
>>
>> static noinline_for_stack
>> @@ -3427,7 +3433,7 @@ int vsscanf(const char *buf, const char *fmt, va_list
>> args)
>> unsigned long long u;
>> } val;
>> s16 field_width;
>> - bool is_sign;
>> + bool is_sign, overflow;
>>
>> while (*fmt) {
>> /* skip any white space in format */
>> @@ -3635,45 +3641,77 @@ int vsscanf(const char *buf, const char *fmt, va_list
>> args)
>> if (is_sign)
>> val.s = simple_strntoll(str,
>> field_width >= 0 ? field_width : INT_MAX,
>> - &next, base);
>> + &next, base, &overflow);
>> else
>> val.u = simple_strntoull(str,
>> field_width >= 0 ? field_width : INT_MAX,
>> - &next, base);
>> + &next, base, &overflow);
>> + if (unlikely(overflow))
>> + return -ERANGE;
>>
>> switch (qualifier) {
>> case 'H': /* that's 'hh' in format */
>> - if (is_sign)
>> + if (is_sign) {
>> + if (unlikely(val.s < SCHAR_MIN || val.s > SCHAR_MAX))
>> + return -ERANGE;
>> *va_arg(args, signed char *) = val.s;
>> - else
>> + } else {
>> + if (unlikely(val.u > UCHAR_MAX))
>> + return -ERANGE;
>> *va_arg(args, unsigned char *) = val.u;
>> + }
>> break;
>> case 'h':
>> - if (is_sign)
>> + if (is_sign) {
>> + if (unlikely(val.s < SHRT_MIN || val.s > SHRT_MAX))
>> + return -ERANGE;
Returning a negative value here will break many existing in-kernel users.
Most users just check for the number of matched elements.
Linus' idea was returning 0 upon overflow (nothing matched) and allowing
overflows (if really needed) by adding a new format string qualifier "!".
e.g. "%!d".
Thanks,
//richard
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow
2023-06-10 2:57 ` [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow Demi Marie Obenour
2023-06-10 7:51 ` Christoph Hellwig
@ 2023-06-10 12:59 ` kernel test robot
1 sibling, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-06-10 12:59 UTC (permalink / raw)
To: Demi Marie Obenour, Dwaipayan Ray, Lukas Bulwahn, Joe Perches,
Jonathan Corbet, Federico Vaga, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Lee Jones,
Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
Rasmus Villemoes
Cc: oe-kbuild-all, Demi Marie Obenour, linux-doc, linux-kernel, xen-devel
Hi Demi,
kernel test robot noticed the following build warnings:
[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on lee-leds/for-leds-next linus/master v6.4-rc5 next-20230609]
[cannot apply to xen-tip/linux-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/vsscanf-Return-ERANGE-on-integer-overflow/20230610-110026
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link: https://lore.kernel.org/r/20230610025759.1813-2-demi%40invisiblethingslab.com
patch subject: [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230610/202306102055.ZSJK8Xsj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
git remote add lee-mfd https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git fetch lee-mfd for-mfd-next
git checkout lee-mfd/for-mfd-next
b4 shazam https://lore.kernel.org/r/20230610025759.1813-2-demi@invisiblethingslab.com
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/staging/media/atomisp/
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306102055.ZSJK8Xsj-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/staging/media/atomisp//pci/ia_css_acc_types.h:25,
from drivers/staging/media/atomisp//pci/ia_css_pipe_public.h:29,
from drivers/staging/media/atomisp//pci/ia_css_stream_public.h:25,
from drivers/staging/media/atomisp//pci/runtime/binary/interface/ia_css_binary.h:23,
from drivers/staging/media/atomisp//pci/runtime/debug/interface/ia_css_debug.h:25,
from drivers/staging/media/atomisp/pci/isp/kernels/macc/macc_1.0/ia_css_macc.host.c:18:
>> drivers/staging/media/atomisp//pci/hive_isp_css_include/platform_support.h:30: warning: "UCHAR_MAX" redefined
30 | #define UCHAR_MAX (255)
|
In file included from include/linux/limits.h:7,
from drivers/staging/media/atomisp//pci/hive_isp_css_include/type_support.h:37,
from drivers/staging/media/atomisp//pci/ia_css_types.h:27,
from drivers/staging/media/atomisp/pci/isp/kernels/macc/macc_1.0/ia_css_macc.host.c:16:
include/vdso/limits.h:5: note: this is the location of the previous definition
5 | #define UCHAR_MAX ((unsigned char)~0U)
|
vim +/UCHAR_MAX +30 drivers/staging/media/atomisp//pci/hive_isp_css_include/platform_support.h
ad85094b293e40 drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h Mauro Carvalho Chehab 2020-04-19 27
ad85094b293e40 drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h Mauro Carvalho Chehab 2020-04-19 28 #define UINT16_MAX USHRT_MAX
ad85094b293e40 drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h Mauro Carvalho Chehab 2020-04-19 29 #define UINT32_MAX UINT_MAX
ad85094b293e40 drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h Mauro Carvalho Chehab 2020-04-19 @30 #define UCHAR_MAX (255)
ad85094b293e40 drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_include/platform_support.h Mauro Carvalho Chehab 2020-04-19 31
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] Add strict version of vsscanf()
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
2023-06-10 2:57 ` [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow Demi Marie Obenour
@ 2023-06-10 2:57 ` Demi Marie Obenour
2023-06-10 7:52 ` Christoph Hellwig
2023-06-10 2:57 ` [PATCH 4/4] Strict XenStore entry parsing Demi Marie Obenour
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Demi Marie Obenour @ 2023-06-10 2:57 UTC (permalink / raw)
To: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes
Cc: Demi Marie Obenour, linux-doc, linux-kernel, xen-devel
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
include/linux/kernel.h | 4 ++++
lib/vsprintf.c | 43 +++++++++++++++++++++++++++++++++++++++---
2 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 0d91e0af01250c1d82f4a2ea562d2619b9cc6e9c..b348b84ce9c4e95031f67e0cbac5de8deca69aac 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -227,8 +227,12 @@ const char *kvasprintf_const(gfp_t gfp, const char *fmt, va_list args);
extern __scanf(2, 3)
int sscanf(const char *, const char *, ...);
+extern __scanf(2, 3)
+int sscanf_strict(const char *, const char *, ...);
extern __scanf(2, 0)
int vsscanf(const char *, const char *, va_list);
+extern __scanf(2, 0)
+int vsscanf_strict(const char *, const char *, va_list);
extern int no_hash_pointers_enable(char *str);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 9846d2385f5b9e8f3945a5664d81047e97cf10d5..2dae357b367e1da8b1004ed6e85e051a045ca36b 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3414,6 +3414,8 @@ EXPORT_SYMBOL_GPL(bprintf);
#endif /* CONFIG_BINARY_PRINTF */
+static int vsscanf_internal(const char *buf, const char *fmt, va_list args, bool strict);
+
/**
* vsscanf - Unformat a buffer into a list of arguments
* @buf: input buffer
@@ -3421,6 +3423,23 @@ EXPORT_SYMBOL_GPL(bprintf);
* @args: arguments
*/
int vsscanf(const char *buf, const char *fmt, va_list args)
+{
+ return vsscanf_internal(buf, fmt, args, false);
+}
+
+/**
+ * vsscanf_strict - Unformat a buffer into a list of arguments, but
+ * do not skip spaces.
+ * @buf: input buffer
+ * @fmt: format of buffer
+ * @args: arguments
+ */
+int vsscanf_strict(const char *buf, const char *fmt, va_list args)
+{
+ return vsscanf_internal(buf, fmt, args, true);
+}
+
+static int vsscanf_internal(const char *buf, const char *fmt, va_list args, bool strict)
{
const char *str = buf;
char *next;
@@ -3530,8 +3549,10 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
char *s = (char *)va_arg(args, char *);
if (field_width == -1)
field_width = SHRT_MAX;
- /* first, skip leading white space in buffer */
- str = skip_spaces(str);
+ if (!strict) {
+ /* first, skip leading white space in buffer */
+ str = skip_spaces(str);
+ }
/* now copy until next white space */
while (*str && !isspace(*str) && field_width--)
@@ -3621,7 +3642,8 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
/* have some sort of integer conversion.
* first, skip white space in buffer.
*/
- str = skip_spaces(str);
+ if (!strict)
+ str = skip_spaces(str);
digit = *str;
if (is_sign && digit == '-') {
@@ -3721,6 +3743,9 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
str = next;
}
+ if (strict && *str)
+ return -EINVAL;
+
return num;
}
EXPORT_SYMBOL(vsscanf);
@@ -3743,3 +3768,15 @@ int sscanf(const char *buf, const char *fmt, ...)
return i;
}
EXPORT_SYMBOL(sscanf);
+int sscanf_strict(const char *buf, const char *fmt, ...)
+{
+ va_list args;
+ int i;
+
+ va_start(args, fmt);
+ i = vsscanf_strict(buf, fmt, args);
+ va_end(args);
+
+ return i;
+}
+EXPORT_SYMBOL(sscanf_strict);
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] Add strict version of vsscanf()
2023-06-10 2:57 ` [PATCH 3/4] Add strict version of vsscanf() Demi Marie Obenour
@ 2023-06-10 7:52 ` Christoph Hellwig
0 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2023-06-10 7:52 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc,
linux-kernel, xen-devel
This needs a real commit message explaining what the strict version
does and why, and why we can't just make the normal version more
strict.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] Strict XenStore entry parsing
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
2023-06-10 2:57 ` [PATCH 2/4] vsscanf(): Return -ERANGE on integer overflow Demi Marie Obenour
2023-06-10 2:57 ` [PATCH 3/4] Add strict version of vsscanf() Demi Marie Obenour
@ 2023-06-10 2:57 ` Demi Marie Obenour
2023-06-10 3:26 ` Matthew Wilcox
2023-06-10 5:42 ` [PATCH 1/4] Rip out simple_strtoll() kernel test robot
` (3 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Demi Marie Obenour @ 2023-06-10 2:57 UTC (permalink / raw)
To: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes
Cc: Demi Marie Obenour, linux-doc, linux-kernel, xen-devel
This uses the newly-introduced strict version of sscanf().
Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
---
drivers/xen/xenbus/xenbus_xs.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index 12e02eb01f5991b31db451cc57037205359b347f..88e94269c9221d16d1a97e59399058e870675729 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -569,16 +569,20 @@ int xenbus_scanf(struct xenbus_transaction t,
const char *dir, const char *node, const char *fmt, ...)
{
va_list ap;
- int ret;
+ int ret = 0;
+ unsigned int len;
char *val;
- val = xenbus_read(t, dir, node, NULL);
+ val = xenbus_read(t, dir, node, &len);
if (IS_ERR(val))
return PTR_ERR(val);
+ if (strlen(val) != len)
+ goto bad;
va_start(ap, fmt);
- ret = vsscanf(val, fmt, ap);
+ ret = vsscanf_strict(val, fmt, ap);
va_end(ap);
+bad:
kfree(val);
/* Distinctive errno. */
if (ret == 0)
@@ -636,15 +640,18 @@ int xenbus_gather(struct xenbus_transaction t, const char *dir, ...)
while (ret == 0 && (name = va_arg(ap, char *)) != NULL) {
const char *fmt = va_arg(ap, char *);
void *result = va_arg(ap, void *);
+ unsigned len;
char *p;
- p = xenbus_read(t, dir, name, NULL);
+ p = xenbus_read(t, dir, name, &len);
if (IS_ERR(p)) {
ret = PTR_ERR(p);
break;
}
- if (fmt) {
- if (sscanf(p, fmt, result) == 0)
+ if (strlen(p) != len)
+ ret = -EINVAL;
+ else if (fmt) {
+ if (sscanf_strict(p, fmt, result) <= 0)
ret = -EINVAL;
kfree(p);
} else
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] Strict XenStore entry parsing
2023-06-10 2:57 ` [PATCH 4/4] Strict XenStore entry parsing Demi Marie Obenour
@ 2023-06-10 3:26 ` Matthew Wilcox
0 siblings, 0 replies; 13+ messages in thread
From: Matthew Wilcox @ 2023-06-10 3:26 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc,
linux-kernel, xen-devel
On Fri, Jun 09, 2023 at 10:57:59PM -0400, Demi Marie Obenour wrote:
> This uses the newly-introduced strict version of sscanf().
I can see that. Why does it do that?
Documentation/process/5.Posting.rst
(in general, there is a lack of detail across all four of these patches
justifying why any of this work is being done. it isn't obvious to me
why skipping leading whitespace is bad in this context)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] Rip out simple_strtoll()
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
` (2 preceding siblings ...)
2023-06-10 2:57 ` [PATCH 4/4] Strict XenStore entry parsing Demi Marie Obenour
@ 2023-06-10 5:42 ` kernel test robot
2023-06-10 5:42 ` kernel test robot
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-06-10 5:42 UTC (permalink / raw)
To: Demi Marie Obenour, Dwaipayan Ray, Lukas Bulwahn, Joe Perches,
Jonathan Corbet, Federico Vaga, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Lee Jones,
Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
Rasmus Villemoes
Cc: llvm, oe-kbuild-all, Demi Marie Obenour, linux-doc, linux-kernel,
xen-devel
Hi Demi,
kernel test robot noticed the following build errors:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.4-rc5 next-20230609]
[cannot apply to xen-tip/linux-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/vsscanf-Return-ERANGE-on-integer-overflow/20230610-110026
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link: https://lore.kernel.org/r/20230610025759.1813-1-demi%40invisiblethingslab.com
patch subject: [PATCH 1/4] Rip out simple_strtoll()
config: arm-randconfig-r046-20230610 (https://download.01.org/0day-ci/archive/20230610/202306101334.lDZERsKo-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add lee-mfd https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git fetch lee-mfd for-mfd-next
git checkout lee-mfd/for-mfd-next
b4 shazam https://lore.kernel.org/r/20230610025759.1813-1-demi@invisiblethingslab.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/md/bcache/
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306101334.lDZERsKo-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/md/bcache/util.c:82:1: error: call to undeclared function 'simple_strtoll'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
STRTO_H(strtoll, long long)
^
drivers/md/bcache/util.c:28:11: note: expanded from macro 'STRTO_H'
type i = simple_ ## name(cp, &e, 10); \
^
<scratch space>:22:1: note: expanded from here
simple_strtoll
^
1 error generated.
vim +/simple_strtoll +82 drivers/md/bcache/util.c
cafe563591446c Kent Overstreet 2013-03-23 79
cafe563591446c Kent Overstreet 2013-03-23 80 STRTO_H(strtoint, int)
cafe563591446c Kent Overstreet 2013-03-23 81 STRTO_H(strtouint, unsigned int)
cafe563591446c Kent Overstreet 2013-03-23 @82 STRTO_H(strtoll, long long)
cafe563591446c Kent Overstreet 2013-03-23 83 STRTO_H(strtoull, unsigned long long)
cafe563591446c Kent Overstreet 2013-03-23 84
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] Rip out simple_strtoll()
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
` (3 preceding siblings ...)
2023-06-10 5:42 ` [PATCH 1/4] Rip out simple_strtoll() kernel test robot
@ 2023-06-10 5:42 ` kernel test robot
2023-06-10 7:49 ` Christoph Hellwig
2023-06-13 10:14 ` Federico Vaga
6 siblings, 0 replies; 13+ messages in thread
From: kernel test robot @ 2023-06-10 5:42 UTC (permalink / raw)
To: Demi Marie Obenour, Dwaipayan Ray, Lukas Bulwahn, Joe Perches,
Jonathan Corbet, Federico Vaga, Juergen Gross,
Stefano Stabellini, Oleksandr Tyshchenko, Lee Jones,
Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino, Petr Mladek,
Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
Rasmus Villemoes
Cc: oe-kbuild-all, Demi Marie Obenour, linux-doc, linux-kernel, xen-devel
Hi Demi,
kernel test robot noticed the following build errors:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on lee-leds/for-leds-next linus/master v6.4-rc5 next-20230609]
[cannot apply to xen-tip/linux-next lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Demi-Marie-Obenour/vsscanf-Return-ERANGE-on-integer-overflow/20230610-110026
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
patch link: https://lore.kernel.org/r/20230610025759.1813-1-demi%40invisiblethingslab.com
patch subject: [PATCH 1/4] Rip out simple_strtoll()
config: csky-randconfig-r011-20230610 (https://download.01.org/0day-ci/archive/20230610/202306101317.YiBrl6OZ-lkp@intel.com/config)
compiler: csky-linux-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add lee-mfd https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git
git fetch lee-mfd for-mfd-next
git checkout lee-mfd/for-mfd-next
b4 shazam https://lore.kernel.org/r/20230610025759.1813-1-demi@invisiblethingslab.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=csky SHELL=/bin/bash drivers/md/bcache/
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306101317.YiBrl6OZ-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/md/bcache/util.c: In function 'bch_strtoll_h':
>> drivers/md/bcache/util.c:28:18: error: implicit declaration of function 'simple_strtoll'; did you mean 'simple_strtoull'? [-Werror=implicit-function-declaration]
28 | type i = simple_ ## name(cp, &e, 10); \
| ^~~~~~~
drivers/md/bcache/util.c:82:1: note: in expansion of macro 'STRTO_H'
82 | STRTO_H(strtoll, long long)
| ^~~~~~~
cc1: some warnings being treated as errors
vim +28 drivers/md/bcache/util.c
cafe563591446c Kent Overstreet 2013-03-23 22
cafe563591446c Kent Overstreet 2013-03-23 23 #define STRTO_H(name, type) \
169ef1cf6171d3 Kent Overstreet 2013-03-28 24 int bch_ ## name ## _h(const char *cp, type *res) \
cafe563591446c Kent Overstreet 2013-03-23 25 { \
cafe563591446c Kent Overstreet 2013-03-23 26 int u = 0; \
cafe563591446c Kent Overstreet 2013-03-23 27 char *e; \
cafe563591446c Kent Overstreet 2013-03-23 @28 type i = simple_ ## name(cp, &e, 10); \
cafe563591446c Kent Overstreet 2013-03-23 29 \
cafe563591446c Kent Overstreet 2013-03-23 30 switch (tolower(*e)) { \
cafe563591446c Kent Overstreet 2013-03-23 31 default: \
cafe563591446c Kent Overstreet 2013-03-23 32 return -EINVAL; \
cafe563591446c Kent Overstreet 2013-03-23 33 case 'y': \
cafe563591446c Kent Overstreet 2013-03-23 34 case 'z': \
cafe563591446c Kent Overstreet 2013-03-23 35 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 36 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 37 case 'e': \
cafe563591446c Kent Overstreet 2013-03-23 38 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 39 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 40 case 'p': \
cafe563591446c Kent Overstreet 2013-03-23 41 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 42 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 43 case 't': \
cafe563591446c Kent Overstreet 2013-03-23 44 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 45 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 46 case 'g': \
cafe563591446c Kent Overstreet 2013-03-23 47 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 48 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 49 case 'm': \
cafe563591446c Kent Overstreet 2013-03-23 50 u++; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 51 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 52 case 'k': \
cafe563591446c Kent Overstreet 2013-03-23 53 u++; \
cafe563591446c Kent Overstreet 2013-03-23 54 if (e++ == cp) \
cafe563591446c Kent Overstreet 2013-03-23 55 return -EINVAL; \
df561f6688fef7 Gustavo A. R. Silva 2020-08-23 56 fallthrough; \
cafe563591446c Kent Overstreet 2013-03-23 57 case '\n': \
cafe563591446c Kent Overstreet 2013-03-23 58 case '\0': \
cafe563591446c Kent Overstreet 2013-03-23 59 if (*e == '\n') \
cafe563591446c Kent Overstreet 2013-03-23 60 e++; \
cafe563591446c Kent Overstreet 2013-03-23 61 } \
cafe563591446c Kent Overstreet 2013-03-23 62 \
cafe563591446c Kent Overstreet 2013-03-23 63 if (*e) \
cafe563591446c Kent Overstreet 2013-03-23 64 return -EINVAL; \
cafe563591446c Kent Overstreet 2013-03-23 65 \
cafe563591446c Kent Overstreet 2013-03-23 66 while (u--) { \
cafe563591446c Kent Overstreet 2013-03-23 67 if ((type) ~0 > 0 && \
cafe563591446c Kent Overstreet 2013-03-23 68 (type) ~0 / 1024 <= i) \
cafe563591446c Kent Overstreet 2013-03-23 69 return -EINVAL; \
cafe563591446c Kent Overstreet 2013-03-23 70 if ((i > 0 && ANYSINT_MAX(type) / 1024 < i) || \
cafe563591446c Kent Overstreet 2013-03-23 71 (i < 0 && -ANYSINT_MAX(type) / 1024 > i)) \
cafe563591446c Kent Overstreet 2013-03-23 72 return -EINVAL; \
cafe563591446c Kent Overstreet 2013-03-23 73 i *= 1024; \
cafe563591446c Kent Overstreet 2013-03-23 74 } \
cafe563591446c Kent Overstreet 2013-03-23 75 \
cafe563591446c Kent Overstreet 2013-03-23 76 *res = i; \
cafe563591446c Kent Overstreet 2013-03-23 77 return 0; \
cafe563591446c Kent Overstreet 2013-03-23 78 } \
cafe563591446c Kent Overstreet 2013-03-23 79
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] Rip out simple_strtoll()
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
` (4 preceding siblings ...)
2023-06-10 5:42 ` kernel test robot
@ 2023-06-10 7:49 ` Christoph Hellwig
2023-06-13 10:14 ` Federico Vaga
6 siblings, 0 replies; 13+ messages in thread
From: Christoph Hellwig @ 2023-06-10 7:49 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Federico Vaga, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Lee Jones, Andy Lutomirski,
Thomas Gleixner, Vincenzo Frascino, Petr Mladek, Steven Rostedt,
Sergey Senozhatsky, Andy Shevchenko, Rasmus Villemoes, linux-doc,
linux-kernel, xen-devel
I'd maybe say remove instead of "rip out", but otherwise this looks
good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/4] Rip out simple_strtoll()
2023-06-10 2:57 [PATCH 1/4] Rip out simple_strtoll() Demi Marie Obenour
` (5 preceding siblings ...)
2023-06-10 7:49 ` Christoph Hellwig
@ 2023-06-13 10:14 ` Federico Vaga
6 siblings, 0 replies; 13+ messages in thread
From: Federico Vaga @ 2023-06-13 10:14 UTC (permalink / raw)
To: Demi Marie Obenour
Cc: Dwaipayan Ray, Lukas Bulwahn, Joe Perches, Jonathan Corbet,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Lee Jones, Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino,
Petr Mladek, Steven Rostedt, Sergey Senozhatsky, Andy Shevchenko,
Rasmus Villemoes, linux-doc, linux-kernel, xen-devel
On 2023-06-10 04:57, Demi Marie Obenour wrote:
> It is not used anywhere but its own unit tests.
>
> Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> ---
> Documentation/dev-tools/checkpatch.rst | 9 ++++-----
> Documentation/process/deprecated.rst | 5 ++---
> .../translations/it_IT/process/deprecated.rst | 9 ++++-----
> .../translations/sp_SP/process/deprecated.rst | 14 +++++++-------
> include/linux/kstrtox.h | 1 -
> lib/kstrtox.c | 2 +-
> lib/test_scanf.c | 10 ----------
> lib/vsprintf.c | 14 --------------
> 8 files changed, 18 insertions(+), 46 deletions(-)
> --- a/Documentation/translations/it_IT/process/deprecated.rst
> +++ b/Documentation/translations/it_IT/process/deprecated.rst
> @@ -118,12 +118,11 @@ Per maggiori dettagli fate riferimento a
> array3_size() e flex_array_size(), ma
> anche le funzioni della famiglia check_mul_overflow(),
> check_add_overflow(),
> check_sub_overflow(), e check_shl_overflow().
>
> -simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull()
> +simple_strtol(), simple_strtoul(), simple_strtoull()
> ----------------------------------------------------------------------
> -Le funzioni simple_strtol(), simple_strtoll(),
> -simple_strtoul(), e simple_strtoull() ignorano volutamente
> -i possibili overflow, e questo può portare il chiamante a generare
> risultati
> -inaspettati. Le rispettive funzioni kstrtol(), kstrtoll(),
> +Le funzioni simple_strtol(), simple_strtoul(), e simple_strtoull()
> ignorano
> +volutamente i possibili overflow, e questo può portare il chiamante a
> generare
> +risultati inaspettati. Le rispettive funzioni kstrtol(), kstrtoll(),
> kstrtoul(), e kstrtoull() sono da considerarsi le corrette
> sostitute; tuttavia va notato che queste richiedono che la stringa sia
> terminata con il carattere NUL o quello di nuova riga.
This is fine
--
Federico Vaga
^ permalink raw reply [flat|nested] 13+ messages in thread