Message ID | 20230610025759.1813-1-demi@invisiblethingslab.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/4] Rip out simple_strtoll() | expand |
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
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
I'd maybe say remove instead of "rip out", but otherwise this looks
good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
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
diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst index c3389c6f38381f038ed5d9a884f2d333a749f8a2..0ae0ca80beb0c0171e8c04306cb5b9ccbc9fa713 100644 --- a/Documentation/dev-tools/checkpatch.rst +++ b/Documentation/dev-tools/checkpatch.rst @@ -290,11 +290,10 @@ API usage See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on **CONSIDER_KSTRTO** - The simple_strtol(), simple_strtoll(), simple_strtoul(), and - simple_strtoull() functions explicitly ignore overflows, which - may lead to unexpected results in callers. The respective kstrtol(), - kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the - correct replacements. + The simple_strtol(), simple_strtoul(), and simple_strtoull() functions + explicitly ignore overflows, which may lead to unexpected results in + callers. The respective kstrtol(), kstrtoll(), kstrtoul(), and kstrtoull() + functions tend to be the correct replacements. See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull diff --git a/Documentation/process/deprecated.rst b/Documentation/process/deprecated.rst index f91b8441f2ef70576c5bad079e631e4077eabed6..b2f677eb6d6b12df63003b00960ef27b1656e4c3 100644 --- a/Documentation/process/deprecated.rst +++ b/Documentation/process/deprecated.rst @@ -109,10 +109,9 @@ For more details, also see array3_size() and flex_array_size(), as well as the related check_mul_overflow(), check_add_overflow(), check_sub_overflow(), and check_shl_overflow() family of functions. -simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull() +simple_strtol(), simple_strtoul(), simple_strtoull() ---------------------------------------------------------------------- -The simple_strtol(), simple_strtoll(), -simple_strtoul(), and simple_strtoull() functions +The simple_strtol(), simple_strtoul(), and simple_strtoull() functions explicitly ignore overflows, which may lead to unexpected results in callers. The respective kstrtol(), kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the diff --git a/Documentation/translations/it_IT/process/deprecated.rst b/Documentation/translations/it_IT/process/deprecated.rst index ba0ed7dc154c95e0e973cb51cc36b95e786079a8..cdc0aa107bce7f4018ad241a02b4f701974e52b4 100644 --- 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. diff --git a/Documentation/translations/sp_SP/process/deprecated.rst b/Documentation/translations/sp_SP/process/deprecated.rst index d52120e0d75354d0d32c33d631f9f364eba32f82..f0ba93e7188f02ae98a8533e2dfee4c82cf78c5c 100644 --- a/Documentation/translations/sp_SP/process/deprecated.rst +++ b/Documentation/translations/sp_SP/process/deprecated.rst @@ -117,14 +117,14 @@ como también la familia de funciones relacionadas check_mul_overflow(), check_add_overflow(), check_sub_overflow(), y check_shl_overflow(). -simple_strtol(), simple_strtoll(), simple_strtoul(), simple_strtoull() +simple_strtol(), simple_strtoul(), simple_strtoull() ---------------------------------------------------------------------- -Las funciones: simple_strtol(), simple_strtoll(), simple_strtoul(), y -simple_strtoull() explícitamente ignoran los desbordamientos, lo que puede -llevar a resultados inesperados por las funciones que las llaman. Las -funciones respectivas kstrtol(), kstrtoll(), kstrtoul(), y kstrtoull() -tienden a ser reemplazos correctos, aunque nótese que necesitarán que la -cadena de caracteres termine en NUL o en el carácter de línea nueva. +Las funciones: simple_strtol(), simple_strtoul(), y simple_strtoull() +explícitamente ignoran los desbordamientos, lo que puede llevar a +resultados inesperados por las funciones que las llaman. Las funciones +respectivas kstrtol(), kstrtoll(), kstrtoul(), y kstrtoull() tienden a +ser reemplazos correctos, aunque nótese que necesitarán que la cadena de +caracteres termine en NUL o en el carácter de línea nueva. strcpy() diff --git a/include/linux/kstrtox.h b/include/linux/kstrtox.h index 529974e22ea799adf5c07b48d803ac9bfc1f85a4..c3ed0ce20be49fba185937e3fd0f949e7be6f9a4 100644 --- a/include/linux/kstrtox.h +++ b/include/linux/kstrtox.h @@ -145,7 +145,6 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t extern unsigned long simple_strtoul(const char *,char **,unsigned int); extern long simple_strtol(const char *,char **,unsigned int); extern unsigned long long simple_strtoull(const char *,char **,unsigned int); -extern long long simple_strtoll(const char *,char **,unsigned int); static inline int strtobool(const char *s, bool *res) { diff --git a/lib/kstrtox.c b/lib/kstrtox.c index 08c14019841af9df6d1c354b4503ecee4c7a9da0..3397a91e07dbad814bb0f2c93d19761a375bedc5 100644 --- a/lib/kstrtox.c +++ b/lib/kstrtox.c @@ -150,7 +150,7 @@ EXPORT_SYMBOL(kstrtoull); * @res: Where to write the result of the conversion on success. * * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error. - * Preferred over simple_strtoll(). Return code must be checked. + * Return code must be checked. */ noinline int kstrtoll(const char *s, unsigned int base, long long *res) diff --git a/lib/test_scanf.c b/lib/test_scanf.c index b620cf7de503542a98f38c45c76e9043b242746e..73e6e912f39c7746b1aaaf35eeb562c9c2d3fac8 100644 --- a/lib/test_scanf.c +++ b/lib/test_scanf.c @@ -727,15 +727,6 @@ static void __init test_simple_strtoull(void) test_simple_strtoxx(unsigned long long, simple_strtoull, "0x%llx", 0); } -static void __init test_simple_strtoll(void) -{ - test_simple_strtoxx(long long, simple_strtoll, "%lld", 10); - test_simple_strtoxx(long long, simple_strtoll, "%lld", 0); - test_simple_strtoxx(long long, simple_strtoll, "%llx", 16); - test_simple_strtoxx(long long, simple_strtoll, "0x%llx", 16); - test_simple_strtoxx(long long, simple_strtoll, "0x%llx", 0); -} - static void __init test_simple_strtoul(void) { test_simple_strtoxx(unsigned long, simple_strtoul, "%lu", 10); @@ -800,7 +791,6 @@ static void __init selftest(void) test_numbers(); test_simple_strtoull(); - test_simple_strtoll(); test_simple_strtoul(); test_simple_strtol(); diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 40f560959b169b4c4ac6154d658cfe76cfd0c5a6..a60d348efb276d66ca07fe464883408df7fdab97 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -144,20 +144,6 @@ static long long simple_strntoll(const char *cp, size_t max_chars, char **endp, return simple_strntoull(cp, max_chars, endp, base); } -/** - * simple_strtoll - convert a string to a signed long long - * @cp: The start of the string - * @endp: A pointer to the end of the parsed string will be placed here - * @base: The number base to use - * - * This function has caveats. Please use kstrtoll instead. - */ -long long simple_strtoll(const char *cp, char **endp, unsigned int base) -{ - return simple_strntoll(cp, INT_MAX, endp, base); -} -EXPORT_SYMBOL(simple_strtoll); - static noinline_for_stack int skip_atoi(const char **s) {
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(-)