* [PATCH] lib/test_string.c: Add test for strlen()
@ 2022-01-30 18:36 Kees Cook
2022-01-30 18:56 ` Andy Shevchenko
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Kees Cook @ 2022-01-30 18:36 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce,
Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm,
linux-hardening
Add a simple test for strlen() functionality, including using it as a
constant expression.
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Peter Rosin <peda@axentia.se>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
I'll be taking this as part of my Clang FORTIFY_SOURCE series.
---
lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/lib/test_string.c b/lib/test_string.c
index 9dfd6f52de92..59994f552c48 100644
--- a/lib/test_string.c
+++ b/lib/test_string.c
@@ -179,6 +179,38 @@ static __init int strnchr_selftest(void)
return 0;
}
+/*
+ * Unlike many other string functions, strlen() can be used in
+ * static initializers when string lengths are known at compile
+ * time. (i.e. Under these conditions, strlen() is a constant
+ * expression.) Make sure it can be used this way.
+ */
+static const int strlen_ce = strlen("tada, a constant expression");
+
+static __init int strlen_selftest(void)
+{
+ /* String length ruler: 123456789012345 */
+ static const char normal[] = "I am normal";
+ static const char *ptr = "where do I go?";
+ static const char trailing[] = "hidden NULLs\0\0\0";
+ static const char leading[] = "\0\0hidden text";
+
+ if (strlen(normal) != 11)
+ return 0x100001;
+ if (strlen(ptr++) != 14)
+ return 0x100002;
+ if (strlen(ptr++) != 13)
+ return 0x100003;
+ if (strlen(trailing) != 12)
+ return 0x100004;
+ if (strlen(leading) != 0)
+ return 0x100005;
+ if (strlen_ce != 27)
+ return 0x100006;
+
+ return 0;
+}
+
static __exit void string_selftest_remove(void)
{
}
@@ -212,6 +244,11 @@ static __init int string_selftest_init(void)
if (subtest)
goto fail;
+ test = 5;
+ subtest = strlen_selftest();
+ if (subtest)
+ goto fail;
+
pr_info("String selftests succeeded\n");
return 0;
fail:
--
2.30.2
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-01-30 18:36 [PATCH] lib/test_string.c: Add test for strlen() Kees Cook @ 2022-01-30 18:56 ` Andy Shevchenko 2022-01-30 20:34 ` Kees Cook 2022-01-30 20:13 ` kernel test robot 2022-02-02 16:01 ` Guenter Roeck 2 siblings, 1 reply; 18+ messages in thread From: Andy Shevchenko @ 2022-01-30 18:56 UTC (permalink / raw) To: Kees Cook Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening On Sun, Jan 30, 2022 at 8:36 PM Kees Cook <keescook@chromium.org> wrote: > > Add a simple test for strlen() functionality, including using it as a > constant expression. ... > +/* > + * Unlike many other string functions, strlen() can be used in > + * static initializers when string lengths are known at compile > + * time. (i.e. Under these conditions, strlen() is a constant > + * expression.) Make sure it can be used this way. > + */ > +static const int strlen_ce = strlen("tada, a constant expression"); So, the compiler will replace this by a constant and then eliminate the condition completely from the code. Did I understand this correctly? > +static __init int strlen_selftest(void) > +{ > + /* String length ruler: 123456789012345 */ > + static const char normal[] = "I am normal"; > + static const char *ptr = "where do I go?"; > + static const char trailing[] = "hidden NULLs\0\0\0"; > + static const char leading[] = "\0\0hidden text"; > + > + if (strlen(normal) != 11) > + return 0x100001; > + if (strlen(ptr++) != 14) > + return 0x100002; > + if (strlen(ptr++) != 13) > + return 0x100003; > + if (strlen(trailing) != 12) > + return 0x100004; > + if (strlen(leading) != 0) > + return 0x100005; > + if (strlen_ce != 27) > + return 0x100006; ...so this part won't ever appear in the assembly (assuming -O2). Same to the rest? If so, why is this not a part of the compiler tests? > + return 0; > +} -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-01-30 18:56 ` Andy Shevchenko @ 2022-01-30 20:34 ` Kees Cook 0 siblings, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-01-30 20:34 UTC (permalink / raw) To: Andy Shevchenko Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening On Sun, Jan 30, 2022 at 08:56:40PM +0200, Andy Shevchenko wrote: > On Sun, Jan 30, 2022 at 8:36 PM Kees Cook <keescook@chromium.org> wrote: > > > > Add a simple test for strlen() functionality, including using it as a > > constant expression. > > ... > > > +/* > > + * Unlike many other string functions, strlen() can be used in > > + * static initializers when string lengths are known at compile > > + * time. (i.e. Under these conditions, strlen() is a constant > > + * expression.) Make sure it can be used this way. > > + */ > > +static const int strlen_ce = strlen("tada, a constant expression"); > > So, the compiler will replace this by a constant and then eliminate > the condition completely from the code. Did I understand this > correctly? Yup! See: https://godbolt.org/z/nTqPaszTh There a few rare places in the kernel that do this, which is how I noticed. (I broke strlen() with the recent FORTIFY changes.) > > +static __init int strlen_selftest(void) > > +{ > > + /* String length ruler: 123456789012345 */ > > + static const char normal[] = "I am normal"; > > + static const char *ptr = "where do I go?"; > > + static const char trailing[] = "hidden NULLs\0\0\0"; > > + static const char leading[] = "\0\0hidden text"; > > + > > + if (strlen(normal) != 11) > > + return 0x100001; > > + if (strlen(ptr++) != 14) > > + return 0x100002; > > + if (strlen(ptr++) != 13) > > + return 0x100003; > > + if (strlen(trailing) != 12) > > + return 0x100004; > > + if (strlen(leading) != 0) > > + return 0x100005; > > > + if (strlen_ce != 27) > > + return 0x100006; > > ...so this part won't ever appear in the assembly (assuming -O2). Correct, unless strlen() breaks. > Same to the rest? If so, why is this not a part of the compiler tests? I wanted to keep everything together -- this includes a macro side-effect test as well ("ptr++"). -Kees -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-01-30 18:36 [PATCH] lib/test_string.c: Add test for strlen() Kees Cook @ 2022-01-30 20:13 ` kernel test robot 2022-01-30 20:13 ` kernel test robot 2022-02-02 16:01 ` Guenter Roeck 2 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2022-01-30 20:13 UTC (permalink / raw) To: Kees Cook Cc: kbuild-all, Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening Hi Kees, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master kees/for-next/pstore v5.17-rc2 next-20220128] [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] url: https://github.com/0day-ci/linux/commits/Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39 config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220131/202201310303.HQlCsvvd-lkp@intel.com/config) compiler: sh4-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/d22fe883339c5141953dbca980b51466ac3e2329 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 git checkout d22fe883339c5141953dbca980b51466ac3e2329 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash M=lib/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> lib/test_string.c:188:30: error: initializer element is not constant 188 | static const int strlen_ce = strlen("tada, a constant expression"); | ^~~~~~ vim +188 lib/test_string.c 181 182 /* 183 * Unlike many other string functions, strlen() can be used in 184 * static initializers when string lengths are known at compile 185 * time. (i.e. Under these conditions, strlen() is a constant 186 * expression.) Make sure it can be used this way. 187 */ > 188 static const int strlen_ce = strlen("tada, a constant expression"); 189 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() @ 2022-01-30 20:13 ` kernel test robot 0 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2022-01-30 20:13 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2366 bytes --] Hi Kees, I love your patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master kees/for-next/pstore v5.17-rc2 next-20220128] [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] url: https://github.com/0day-ci/linux/commits/Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39 config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220131/202201310303.HQlCsvvd-lkp(a)intel.com/config) compiler: sh4-linux-gcc (GCC) 11.2.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/d22fe883339c5141953dbca980b51466ac3e2329 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 git checkout d22fe883339c5141953dbca980b51466ac3e2329 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash M=lib/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> lib/test_string.c:188:30: error: initializer element is not constant 188 | static const int strlen_ce = strlen("tada, a constant expression"); | ^~~~~~ vim +188 lib/test_string.c 181 182 /* 183 * Unlike many other string functions, strlen() can be used in 184 * static initializers when string lengths are known at compile 185 * time. (i.e. Under these conditions, strlen() is a constant 186 * expression.) Make sure it can be used this way. 187 */ > 188 static const int strlen_ce = strlen("tada, a constant expression"); 189 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-01-30 20:13 ` kernel test robot @ 2022-01-30 20:35 ` Kees Cook -1 siblings, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-01-30 20:35 UTC (permalink / raw) To: kernel test robot Cc: kbuild-all, Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening On Mon, Jan 31, 2022 at 04:13:30AM +0800, kernel test robot wrote: > Hi Kees, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linux/master] > [also build test ERROR on linus/master kees/for-next/pstore v5.17-rc2 next-20220128] > [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] > > url: https://github.com/0day-ci/linux/commits/Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39 > config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220131/202201310303.HQlCsvvd-lkp@intel.com/config) > compiler: sh4-linux-gcc (GCC) 11.2.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/0day-ci/linux/commit/d22fe883339c5141953dbca980b51466ac3e2329 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 > git checkout d22fe883339c5141953dbca980b51466ac3e2329 > # save the config file to linux build tree > mkdir build_dir > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash M=lib/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > >> lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > | ^~~~~~ Thanks; this is expected -- the strlen() in -next is currently broken (and will be fixed in the next -next). -Kees > > > vim +188 lib/test_string.c > > 181 > 182 /* > 183 * Unlike many other string functions, strlen() can be used in > 184 * static initializers when string lengths are known at compile > 185 * time. (i.e. Under these conditions, strlen() is a constant > 186 * expression.) Make sure it can be used this way. > 187 */ > > 188 static const int strlen_ce = strlen("tada, a constant expression"); > 189 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() @ 2022-01-30 20:35 ` Kees Cook 0 siblings, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-01-30 20:35 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 2676 bytes --] On Mon, Jan 31, 2022 at 04:13:30AM +0800, kernel test robot wrote: > Hi Kees, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linux/master] > [also build test ERROR on linus/master kees/for-next/pstore v5.17-rc2 next-20220128] > [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] > > url: https://github.com/0day-ci/linux/commits/Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 > base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2c271fe77d52a0555161926c232cd5bc07178b39 > config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220131/202201310303.HQlCsvvd-lkp(a)intel.com/config) > compiler: sh4-linux-gcc (GCC) 11.2.0 > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # https://github.com/0day-ci/linux/commit/d22fe883339c5141953dbca980b51466ac3e2329 > git remote add linux-review https://github.com/0day-ci/linux > git fetch --no-tags linux-review Kees-Cook/lib-test_string-c-Add-test-for-strlen/20220131-023726 > git checkout d22fe883339c5141953dbca980b51466ac3e2329 > # save the config file to linux build tree > mkdir build_dir > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=sh SHELL=/bin/bash M=lib/ > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All errors (new ones prefixed by >>): > > >> lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > | ^~~~~~ Thanks; this is expected -- the strlen() in -next is currently broken (and will be fixed in the next -next). -Kees > > > vim +188 lib/test_string.c > > 181 > 182 /* > 183 * Unlike many other string functions, strlen() can be used in > 184 * static initializers when string lengths are known at compile > 185 * time. (i.e. Under these conditions, strlen() is a constant > 186 * expression.) Make sure it can be used this way. > 187 */ > > 188 static const int strlen_ce = strlen("tada, a constant expression"); > 189 > > --- > 0-DAY CI Kernel Test Service, Intel Corporation > https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-01-30 18:36 [PATCH] lib/test_string.c: Add test for strlen() Kees Cook 2022-01-30 18:56 ` Andy Shevchenko 2022-01-30 20:13 ` kernel test robot @ 2022-02-02 16:01 ` Guenter Roeck 2022-02-02 16:16 ` Andy Shevchenko 2022-02-02 20:52 ` Kees Cook 2 siblings, 2 replies; 18+ messages in thread From: Guenter Roeck @ 2022-02-02 16:01 UTC (permalink / raw) To: Kees Cook Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: > Add a simple test for strlen() functionality, including using it as a > constant expression. > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: Peter Rosin <peda@axentia.se> > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > I'll be taking this as part of my Clang FORTIFY_SOURCE series. > --- > lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/lib/test_string.c b/lib/test_string.c > index 9dfd6f52de92..59994f552c48 100644 > --- a/lib/test_string.c > +++ b/lib/test_string.c > @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void) > return 0; > } > > +/* > + * Unlike many other string functions, strlen() can be used in > + * static initializers when string lengths are known at compile > + * time. (i.e. Under these conditions, strlen() is a constant > + * expression.) Make sure it can be used this way. > + */ > +static const int strlen_ce = strlen("tada, a constant expression"); > + This results in: lib/test_string.c:188:30: error: initializer element is not constant 188 | static const int strlen_ce = strlen("tada, a constant expression"); for several of my tests. I don't think you can mandate that a compiler implements this. Guenter > +static __init int strlen_selftest(void) > +{ > + /* String length ruler: 123456789012345 */ > + static const char normal[] = "I am normal"; > + static const char *ptr = "where do I go?"; > + static const char trailing[] = "hidden NULLs\0\0\0"; > + static const char leading[] = "\0\0hidden text"; > + > + if (strlen(normal) != 11) > + return 0x100001; > + if (strlen(ptr++) != 14) > + return 0x100002; > + if (strlen(ptr++) != 13) > + return 0x100003; > + if (strlen(trailing) != 12) > + return 0x100004; > + if (strlen(leading) != 0) > + return 0x100005; > + if (strlen_ce != 27) > + return 0x100006; > + > + return 0; > +} > + > static __exit void string_selftest_remove(void) > { > } > @@ -212,6 +244,11 @@ static __init int string_selftest_init(void) > if (subtest) > goto fail; > > + test = 5; > + subtest = strlen_selftest(); > + if (subtest) > + goto fail; > + > pr_info("String selftests succeeded\n"); > return 0; > fail: > -- > 2.30.2 > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-02 16:01 ` Guenter Roeck @ 2022-02-02 16:16 ` Andy Shevchenko 2022-02-02 20:52 ` Kees Cook 1 sibling, 0 replies; 18+ messages in thread From: Andy Shevchenko @ 2022-02-02 16:16 UTC (permalink / raw) To: Guenter Roeck Cc: Kees Cook, Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote: > On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: ... > > +static const int strlen_ce = strlen("tada, a constant expression"); > > This results in: > > lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > for several of my tests. I don't think you can mandate that a compiler > implements this. With -O2 probably one can, otherwise I agree. -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-02 16:01 ` Guenter Roeck 2022-02-02 16:16 ` Andy Shevchenko @ 2022-02-02 20:52 ` Kees Cook 2022-02-02 23:12 ` Guenter Roeck 1 sibling, 1 reply; 18+ messages in thread From: Kees Cook @ 2022-02-02 20:52 UTC (permalink / raw) To: Guenter Roeck Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote: > On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: > > Add a simple test for strlen() functionality, including using it as a > > constant expression. > > > > Cc: Geert Uytterhoeven <geert@linux-m68k.org> > > Cc: Peter Rosin <peda@axentia.se> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > --- > > I'll be taking this as part of my Clang FORTIFY_SOURCE series. > > --- > > lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/lib/test_string.c b/lib/test_string.c > > index 9dfd6f52de92..59994f552c48 100644 > > --- a/lib/test_string.c > > +++ b/lib/test_string.c > > @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void) > > return 0; > > } > > > > +/* > > + * Unlike many other string functions, strlen() can be used in > > + * static initializers when string lengths are known at compile > > + * time. (i.e. Under these conditions, strlen() is a constant > > + * expression.) Make sure it can be used this way. > > + */ > > +static const int strlen_ce = strlen("tada, a constant expression"); > > + > > This results in: > > lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > for several of my tests. I don't think you can mandate that a compiler > implements this. Which tests? This property of strlen() is already required by our builds (this is how I tripped over it). For example: drivers/firmware/xilinx/zynqmp-debug.c: #define PM_API(id) {id, #id, strlen(#id)} static struct pm_api_info pm_api_list[] = { PM_API(PM_GET_API_VERSION), PM_API(PM_QUERY_DATA), }; > > Guenter > > > +static __init int strlen_selftest(void) > > +{ > > + /* String length ruler: 123456789012345 */ > > + static const char normal[] = "I am normal"; > > + static const char *ptr = "where do I go?"; > > + static const char trailing[] = "hidden NULLs\0\0\0"; > > + static const char leading[] = "\0\0hidden text"; > > + > > + if (strlen(normal) != 11) > > + return 0x100001; > > + if (strlen(ptr++) != 14) > > + return 0x100002; > > + if (strlen(ptr++) != 13) > > + return 0x100003; > > + if (strlen(trailing) != 12) > > + return 0x100004; > > + if (strlen(leading) != 0) > > + return 0x100005; > > + if (strlen_ce != 27) > > + return 0x100006; > > + > > + return 0; > > +} > > + > > static __exit void string_selftest_remove(void) > > { > > } > > @@ -212,6 +244,11 @@ static __init int string_selftest_init(void) > > if (subtest) > > goto fail; > > > > + test = 5; > > + subtest = strlen_selftest(); > > + if (subtest) > > + goto fail; > > + > > pr_info("String selftests succeeded\n"); > > return 0; > > fail: > > -- > > 2.30.2 > > -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-02 20:52 ` Kees Cook @ 2022-02-02 23:12 ` Guenter Roeck 2022-02-03 8:04 ` Geert Uytterhoeven 0 siblings, 1 reply; 18+ messages in thread From: Guenter Roeck @ 2022-02-02 23:12 UTC (permalink / raw) To: Kees Cook Cc: Geert Uytterhoeven, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, linux-kernel, llvm, linux-hardening On 2/2/22 12:52, Kees Cook wrote: > On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote: >> On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: >>> Add a simple test for strlen() functionality, including using it as a >>> constant expression. >>> >>> Cc: Geert Uytterhoeven <geert@linux-m68k.org> >>> Cc: Peter Rosin <peda@axentia.se> >>> Signed-off-by: Kees Cook <keescook@chromium.org> >>> --- >>> I'll be taking this as part of my Clang FORTIFY_SOURCE series. >>> --- >>> lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 37 insertions(+) >>> >>> diff --git a/lib/test_string.c b/lib/test_string.c >>> index 9dfd6f52de92..59994f552c48 100644 >>> --- a/lib/test_string.c >>> +++ b/lib/test_string.c >>> @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void) >>> return 0; >>> } >>> >>> +/* >>> + * Unlike many other string functions, strlen() can be used in >>> + * static initializers when string lengths are known at compile >>> + * time. (i.e. Under these conditions, strlen() is a constant >>> + * expression.) Make sure it can be used this way. >>> + */ >>> +static const int strlen_ce = strlen("tada, a constant expression"); >>> + >> >> This results in: >> >> lib/test_string.c:188:30: error: initializer element is not constant >> 188 | static const int strlen_ce = strlen("tada, a constant expression"); >> >> for several of my tests. I don't think you can mandate that a compiler >> implements this. > > Which tests? > Some examples: Build reference: next-20220202 Compiler version: m68k-linux-gcc (GCC) 11.2.0 Building m68k:defconfig ... failed -------------- Error log: lib/test_string.c:188:30: error: initializer element is not constant 188 | static const int strlen_ce = strlen("tada, a constant expression"); Building mips:malta_defconfig:nocd:smp:net,e1000:initrd ... failed ------------ Error log: lib/test_string.c:188:30: error: initializer element is not constant static const int strlen_ce = strlen("tada, a constant expression"); Building i386:q35:Broadwell:defconfig:smp:ata:net,rtl8139:hd ... failed ------------ Error log: lib/test_string.c:188:30: error: initializer element is not constant 188 | static const int strlen_ce = strlen("tada, a constant expression"); i386 and is defconfig + CONFIG_STRING_SELFTEST=y; mips is malta_defconfig + CONFIG_STRING_SELFTEST=y. All use gcc 11.2. There may be more, but there are so many failures in -next right now that I may be missing some. > This property of strlen() is already required by our builds (this is how > I tripped over it). For example: > > drivers/firmware/xilinx/zynqmp-debug.c: > > #define PM_API(id) {id, #id, strlen(#id)} > static struct pm_api_info pm_api_list[] = { > PM_API(PM_GET_API_VERSION), > PM_API(PM_QUERY_DATA), > }; I do not think that it is a C standard that strlen() on a constant string must be compile-time evaluated and result in a constant. Anyway, key difference, I think, is the presence of an architecture-specific version of strlen(), or the maybe non-presence of __HAVE_ARCH_STRLEN, or the definition of strlen() in include/linux/fortify-string.h. Guenter ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-02 23:12 ` Guenter Roeck @ 2022-02-03 8:04 ` Geert Uytterhoeven 2022-02-03 16:41 ` Kees Cook 2022-02-03 17:15 ` Kees Cook 0 siblings, 2 replies; 18+ messages in thread From: Geert Uytterhoeven @ 2022-02-03 8:04 UTC (permalink / raw) To: Guenter Roeck Cc: Kees Cook, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening Hi Günter, On Thu, Feb 3, 2022 at 12:12 AM Guenter Roeck <linux@roeck-us.net> wrote: > On 2/2/22 12:52, Kees Cook wrote: > > On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote: > >> On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: > >>> Add a simple test for strlen() functionality, including using it as a > >>> constant expression. > >>> > >>> Cc: Geert Uytterhoeven <geert@linux-m68k.org> > >>> Cc: Peter Rosin <peda@axentia.se> > >>> Signed-off-by: Kees Cook <keescook@chromium.org> > >>> --- > >>> I'll be taking this as part of my Clang FORTIFY_SOURCE series. > >>> --- > >>> lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 37 insertions(+) > >>> > >>> diff --git a/lib/test_string.c b/lib/test_string.c > >>> index 9dfd6f52de92..59994f552c48 100644 > >>> --- a/lib/test_string.c > >>> +++ b/lib/test_string.c > >>> @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void) > >>> return 0; > >>> } > >>> > >>> +/* > >>> + * Unlike many other string functions, strlen() can be used in > >>> + * static initializers when string lengths are known at compile > >>> + * time. (i.e. Under these conditions, strlen() is a constant > >>> + * expression.) Make sure it can be used this way. > >>> + */ > >>> +static const int strlen_ce = strlen("tada, a constant expression"); > >>> + > >> > >> This results in: > >> > >> lib/test_string.c:188:30: error: initializer element is not constant > >> 188 | static const int strlen_ce = strlen("tada, a constant expression"); > >> > >> for several of my tests. I don't think you can mandate that a compiler > >> implements this. > > > > Which tests? > > > > Some examples: > > Build reference: next-20220202 > Compiler version: m68k-linux-gcc (GCC) 11.2.0 > > Building m68k:defconfig ... failed > -------------- > Error log: > lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > Building mips:malta_defconfig:nocd:smp:net,e1000:initrd ... failed > ------------ > Error log: > lib/test_string.c:188:30: error: initializer element is not constant > static const int strlen_ce = strlen("tada, a constant expression"); > > Building i386:q35:Broadwell:defconfig:smp:ata:net,rtl8139:hd ... failed > ------------ > Error log: > lib/test_string.c:188:30: error: initializer element is not constant > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > i386 and is defconfig + CONFIG_STRING_SELFTEST=y; mips is > malta_defconfig + CONFIG_STRING_SELFTEST=y. All use gcc 11.2. > > There may be more, but there are so many failures in -next right now > that I may be missing some. > > > This property of strlen() is already required by our builds (this is how > > I tripped over it). For example: > > > > drivers/firmware/xilinx/zynqmp-debug.c: > > > > #define PM_API(id) {id, #id, strlen(#id)} > > static struct pm_api_info pm_api_list[] = { > > PM_API(PM_GET_API_VERSION), > > PM_API(PM_QUERY_DATA), > > }; > > I do not think that it is a C standard that strlen() on a constant string > must be compile-time evaluated and result in a constant. Not if -ffreestanding, which is what several architectures are using nowadays, to a.o. prevent gcc from replacing calls to stdlib functions to other stdlib functions (e.g. strncat() -> strlen() + store, strncmp() -> strcmp()), which breaks linking if the latter is only provided inline. > Anyway, key difference, I think, is the presence of an architecture-specific > version of strlen(), or the maybe non-presence of __HAVE_ARCH_STRLEN, > or the definition of strlen() in include/linux/fortify-string.h. It works after dropping -ffreestanding. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 8:04 ` Geert Uytterhoeven @ 2022-02-03 16:41 ` Kees Cook 2022-02-03 17:15 ` Kees Cook 1 sibling, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-02-03 16:41 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote: > Hi Günter, > > On Thu, Feb 3, 2022 at 12:12 AM Guenter Roeck <linux@roeck-us.net> wrote: > > On 2/2/22 12:52, Kees Cook wrote: > > > On Wed, Feb 02, 2022 at 08:01:49AM -0800, Guenter Roeck wrote: > > >> On Sun, Jan 30, 2022 at 10:36:53AM -0800, Kees Cook wrote: > > >>> Add a simple test for strlen() functionality, including using it as a > > >>> constant expression. > > >>> > > >>> Cc: Geert Uytterhoeven <geert@linux-m68k.org> > > >>> Cc: Peter Rosin <peda@axentia.se> > > >>> Signed-off-by: Kees Cook <keescook@chromium.org> > > >>> --- > > >>> I'll be taking this as part of my Clang FORTIFY_SOURCE series. > > >>> --- > > >>> lib/test_string.c | 37 +++++++++++++++++++++++++++++++++++++ > > >>> 1 file changed, 37 insertions(+) > > >>> > > >>> diff --git a/lib/test_string.c b/lib/test_string.c > > >>> index 9dfd6f52de92..59994f552c48 100644 > > >>> --- a/lib/test_string.c > > >>> +++ b/lib/test_string.c > > >>> @@ -179,6 +179,38 @@ static __init int strnchr_selftest(void) > > >>> return 0; > > >>> } > > >>> > > >>> +/* > > >>> + * Unlike many other string functions, strlen() can be used in > > >>> + * static initializers when string lengths are known at compile > > >>> + * time. (i.e. Under these conditions, strlen() is a constant > > >>> + * expression.) Make sure it can be used this way. > > >>> + */ > > >>> +static const int strlen_ce = strlen("tada, a constant expression"); > > >>> + > > >> > > >> This results in: > > >> > > >> lib/test_string.c:188:30: error: initializer element is not constant > > >> 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > >> > > >> for several of my tests. I don't think you can mandate that a compiler > > >> implements this. > > > > > > Which tests? > > > > > > > Some examples: > > > > Build reference: next-20220202 > > Compiler version: m68k-linux-gcc (GCC) 11.2.0 > > > > Building m68k:defconfig ... failed > > -------------- > > Error log: > > lib/test_string.c:188:30: error: initializer element is not constant > > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > > > Building mips:malta_defconfig:nocd:smp:net,e1000:initrd ... failed > > ------------ > > Error log: > > lib/test_string.c:188:30: error: initializer element is not constant > > static const int strlen_ce = strlen("tada, a constant expression"); > > > > Building i386:q35:Broadwell:defconfig:smp:ata:net,rtl8139:hd ... failed > > ------------ > > Error log: > > lib/test_string.c:188:30: error: initializer element is not constant > > 188 | static const int strlen_ce = strlen("tada, a constant expression"); > > > > i386 and is defconfig + CONFIG_STRING_SELFTEST=y; mips is > > malta_defconfig + CONFIG_STRING_SELFTEST=y. All use gcc 11.2. > > > > There may be more, but there are so many failures in -next right now > > that I may be missing some. > > > > > This property of strlen() is already required by our builds (this is how > > > I tripped over it). For example: > > > > > > drivers/firmware/xilinx/zynqmp-debug.c: > > > > > > #define PM_API(id) {id, #id, strlen(#id)} > > > static struct pm_api_info pm_api_list[] = { > > > PM_API(PM_GET_API_VERSION), > > > PM_API(PM_QUERY_DATA), > > > }; > > > > I do not think that it is a C standard that strlen() on a constant string > > must be compile-time evaluated and result in a constant. > > Not if -ffreestanding, which is what several architectures are > using nowadays, to a.o. prevent gcc from replacing calls to stdlib > functions to other stdlib functions (e.g. strncat() -> strlen() + > store, strncmp() -> strcmp()), which breaks linking if the latter is > only provided inline. > > > Anyway, key difference, I think, is the presence of an architecture-specific > > version of strlen(), or the maybe non-presence of __HAVE_ARCH_STRLEN, > > or the definition of strlen() in include/linux/fortify-string.h. > > It works after dropping -ffreestanding. Ah-ha, thanks for the clue here. I'll see if there is some way to detect this (or, I guess, drop the patch). I don't like that this requirement depends on the architecture, so I'd rather it behave the same everywhere. Hmm -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 8:04 ` Geert Uytterhoeven 2022-02-03 16:41 ` Kees Cook @ 2022-02-03 17:15 ` Kees Cook 2022-02-03 18:09 ` Geert Uytterhoeven 1 sibling, 1 reply; 18+ messages in thread From: Kees Cook @ 2022-02-03 17:15 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote: > Not if -ffreestanding, which is what several architectures are > using nowadays, to a.o. prevent gcc from replacing calls to stdlib > functions to other stdlib functions (e.g. strncat() -> strlen() + > store, strncmp() -> strcmp()), which breaks linking if the latter is > only provided inline. Hah, for i386: arch/x86/Makefile # temporary until string.h is fixed KBUILD_CFLAGS += -ffreestanding This "temporary" is from 2006. ;) 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to builtin") Removing that appears to solve it, and appears to build correctly. I'll continue testing. > It works after dropping -ffreestanding. I wonder if the other architectures were just copying x86? -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 17:15 ` Kees Cook @ 2022-02-03 18:09 ` Geert Uytterhoeven 2022-02-03 19:50 ` Nick Desaulniers 0 siblings, 1 reply; 18+ messages in thread From: Geert Uytterhoeven @ 2022-02-03 18:09 UTC (permalink / raw) To: Kees Cook Cc: Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Nick Desaulniers, Linux Kernel Mailing List, llvm, linux-hardening Hi Kees, On Thu, Feb 3, 2022 at 6:15 PM Kees Cook <keescook@chromium.org> wrote: > On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote: > > Not if -ffreestanding, which is what several architectures are > > using nowadays, to a.o. prevent gcc from replacing calls to stdlib > > functions to other stdlib functions (e.g. strncat() -> strlen() + > > store, strncmp() -> strcmp()), which breaks linking if the latter is > > only provided inline. > > Hah, for i386: > > arch/x86/Makefile > # temporary until string.h is fixed > KBUILD_CFLAGS += -ffreestanding > > This "temporary" is from 2006. ;) And before that, we had it in the main Makefile. > 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to builtin") > > Removing that appears to solve it, and appears to build correctly. I'll > continue testing. > > > It works after dropping -ffreestanding. > > I wonder if the other architectures were just copying x86? At least on m68k it was added because gcc added new optimizations that broke if an architecture provides some functions as inline. As the kernel is not supposed to be linked with the standard C library, -ffreestanding should be correct, isn't it? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 18:09 ` Geert Uytterhoeven @ 2022-02-03 19:50 ` Nick Desaulniers 2022-02-03 20:25 ` Kees Cook 2022-02-03 20:45 ` Kees Cook 0 siblings, 2 replies; 18+ messages in thread From: Nick Desaulniers @ 2022-02-03 19:50 UTC (permalink / raw) To: Geert Uytterhoeven, Kees Cook Cc: Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Linux Kernel Mailing List, llvm, linux-hardening On Thu, Feb 3, 2022 at 10:10 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Kees, > > On Thu, Feb 3, 2022 at 6:15 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote: > > > Not if -ffreestanding, which is what several architectures are > > > using nowadays, to a.o. prevent gcc from replacing calls to stdlib > > > functions to other stdlib functions (e.g. strncat() -> strlen() + > > > store, strncmp() -> strcmp()), which breaks linking if the latter is > > > only provided inline. > > > > Hah, for i386: > > > > arch/x86/Makefile > > # temporary until string.h is fixed > > KBUILD_CFLAGS += -ffreestanding > > > > This "temporary" is from 2006. ;)\ IIRC I sent a patch removing that. Yeah, Kees even signed off on it. https://lore.kernel.org/lkml/20200817220212.338670-5-ndesaulniers@google.com/#t I still think that's the right way to go, perhaps worth a resend to rekick discussions. > > And before that, we had it in the main Makefile. > > > 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to builtin") > > > > Removing that appears to solve it, and appears to build correctly. I'll > > continue testing. > > > > > It works after dropping -ffreestanding. > > > > I wonder if the other architectures were just copying x86? > > At least on m68k it was added because gcc added new optimizations > that broke if an architecture provides some functions as inline. > As the kernel is not supposed to be linked with the standard C > library, -ffreestanding should be correct, isn't it? The kernel does not link against a libc; but it does provide many symbols that libc would provide, with the same or similar enough semantics that I would strongly recommend we _don't_ use -ffreestanding in order to get such libcall optimizations (there are a lot; see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp for some examples) and simply use -fno-builtin-* when necessary, or fix the kernel implementations individually. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 19:50 ` Nick Desaulniers @ 2022-02-03 20:25 ` Kees Cook 2022-02-03 20:45 ` Kees Cook 1 sibling, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-02-03 20:25 UTC (permalink / raw) To: Nick Desaulniers Cc: Geert Uytterhoeven, Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Linux Kernel Mailing List, llvm, linux-hardening On Thu, Feb 03, 2022 at 11:50:34AM -0800, Nick Desaulniers wrote: > On Thu, Feb 3, 2022 at 10:10 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Kees, > > > > On Thu, Feb 3, 2022 at 6:15 PM Kees Cook <keescook@chromium.org> wrote: > > > On Thu, Feb 03, 2022 at 09:04:22AM +0100, Geert Uytterhoeven wrote: > > > > Not if -ffreestanding, which is what several architectures are > > > > using nowadays, to a.o. prevent gcc from replacing calls to stdlib > > > > functions to other stdlib functions (e.g. strncat() -> strlen() + > > > > store, strncmp() -> strcmp()), which breaks linking if the latter is > > > > only provided inline. > > > > > > Hah, for i386: > > > > > > arch/x86/Makefile > > > # temporary until string.h is fixed > > > KBUILD_CFLAGS += -ffreestanding > > > > > > This "temporary" is from 2006. ;)\ > > IIRC I sent a patch removing that. Yeah, Kees even signed off on it. > https://lore.kernel.org/lkml/20200817220212.338670-5-ndesaulniers@google.com/#t > I still think that's the right way to go, perhaps worth a resend to > rekick discussions. Hah. Yay. I'll go pick this into the memcpy topic branch. Building x86_64 and ia32 differently makes no sense (and this solves the head-scratching compile-time test failures I was seeing on ia32 too). > > > > And before that, we had it in the main Makefile. > > > > > 6edfba1b33c7 ("[PATCH] x86_64: Don't define string functions to builtin") > > > > > > Removing that appears to solve it, and appears to build correctly. I'll > > > continue testing. > > > > > > > It works after dropping -ffreestanding. > > > > > > I wonder if the other architectures were just copying x86? > > > > At least on m68k it was added because gcc added new optimizations > > that broke if an architecture provides some functions as inline. > > As the kernel is not supposed to be linked with the standard C > > library, -ffreestanding should be correct, isn't it? > > The kernel does not link against a libc; but it does provide many > symbols that libc would provide, with the same or similar enough > semantics that I would strongly recommend we _don't_ use > -ffreestanding in order to get such libcall optimizations (there are a > lot; see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp > for some examples) and simply use -fno-builtin-* when necessary, or > fix the kernel implementations individually. Right, so, I think for x86 it's straight forward. For the other architectures it may need more careful checking, so I'm just going to drop the new test from the memcpy topic branch, and maybe start a "-ffreestanding removal" topic branch so there isn't a cross-dependency here. -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] lib/test_string.c: Add test for strlen() 2022-02-03 19:50 ` Nick Desaulniers 2022-02-03 20:25 ` Kees Cook @ 2022-02-03 20:45 ` Kees Cook 1 sibling, 0 replies; 18+ messages in thread From: Kees Cook @ 2022-02-03 20:45 UTC (permalink / raw) To: Nick Desaulniers Cc: Geert Uytterhoeven, Guenter Roeck, Peter Rosin, Andy Shevchenko, Matteo Croce, Nathan Chancellor, Linux Kernel Mailing List, llvm, linux-hardening On Thu, Feb 03, 2022 at 11:50:34AM -0800, Nick Desaulniers wrote: > The kernel does not link against a libc; but it does provide many > symbols that libc would provide, with the same or similar enough > semantics that I would strongly recommend we _don't_ use > -ffreestanding in order to get such libcall optimizations (there are a > lot; see https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp > for some examples) and simply use -fno-builtin-* when necessary, or > fix the kernel implementations individually. Right, we really don't want -ffreestanding. Rather, we want to not link against libgcc. This is mostly true already, though some of the smaller architectures still do: $ git grep print-libgcc arch/arc/Makefile:LIBGCC = $(shell $(CC) $(cflags-y) --print-libgcc-file-name) arch/csky/Makefile: $(shell $(CC) $(KBUILD_CFLAGS) $(KCFLAGS) -print-libgcc-file-name) arch/h8300/boot/compressed/Makefile:LIBGCC := $(shell $(CROSS-COMPILE)$(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name 2>/dev/null) arch/nios2/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) $(KCFLAGS) -print-libgcc-file-name) arch/openrisc/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/parisc/Makefile:LIBGCC := $(shell $(CC) -print-libgcc-file-name) arch/xtensa/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) arch/xtensa/boot/boot-redboot/Makefile:LIBGCC := $(shell $(CC) $(KBUILD_CFLAGS) -print-libgcc-file-name) -- Kees Cook ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2022-02-03 20:45 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-30 18:36 [PATCH] lib/test_string.c: Add test for strlen() Kees Cook 2022-01-30 18:56 ` Andy Shevchenko 2022-01-30 20:34 ` Kees Cook 2022-01-30 20:13 ` kernel test robot 2022-01-30 20:13 ` kernel test robot 2022-01-30 20:35 ` Kees Cook 2022-01-30 20:35 ` Kees Cook 2022-02-02 16:01 ` Guenter Roeck 2022-02-02 16:16 ` Andy Shevchenko 2022-02-02 20:52 ` Kees Cook 2022-02-02 23:12 ` Guenter Roeck 2022-02-03 8:04 ` Geert Uytterhoeven 2022-02-03 16:41 ` Kees Cook 2022-02-03 17:15 ` Kees Cook 2022-02-03 18:09 ` Geert Uytterhoeven 2022-02-03 19:50 ` Nick Desaulniers 2022-02-03 20:25 ` Kees Cook 2022-02-03 20:45 ` Kees Cook
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.