* Re: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
2023-04-08 0:33 ` [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows kernel test robot
@ 2023-04-18 18:27 ` Nick Desaulniers
0 siblings, 0 replies; 2+ messages in thread
From: Nick Desaulniers @ 2023-04-18 18:27 UTC (permalink / raw)
To: kernel test robot
Cc: Kees Cook, linux-hardening, oe-kbuild-all, Andy Shevchenko,
Cezary Rojewski, Puyou Lu, Mark Brown, Josh Poimboeuf,
Peter Zijlstra, Brendan Higgins, David Gow, Andrew Morton,
Linux Memory Management List, Nathan Chancellor,
Alexander Potapenko, Zhaoyang Huang, Randy Dunlap,
Geert Uytterhoeven, Miguel Ojeda, Alexander Lobakin,
Liam Howlett, Vlastimil Babka, Dan Williams, Rasmus Villemoes,
Yury Norov, Jason A. Donenfeld, Sander Vanheule, Eric Biggers,
Masami Hiramatsu (Google),
Andrey Konovalov
On Fri, Apr 7, 2023 at 5:33 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Kees,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on kees/for-next/hardening]
> [also build test WARNING on kees/for-next/pstore kees/for-next/kspp linus/master tip/x86/core v6.3-rc5 next-20230406]
> [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/Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
> base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
> patch link: https://lore.kernel.org/r/20230407192717.636137-9-keescook%40chromium.org
> patch subject: [PATCH v2 09/10] fortify: Add KUnit tests for runtime overflows
> config: openrisc-randconfig-r034-20230405 (https://download.01.org/0day-ci/archive/20230408/202304080811.nYP4KpPZ-lkp@intel.com/config)
> compiler: or1k-linux-gcc (GCC) 12.1.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/intel-lab-lkp/linux/commit/d212962ef7682ee160bf38fa455475558f031759
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Kees-Cook/kunit-tool-Enable-CONFIG_FORTIFY_SOURCE-under-UML/20230408-032959
> git checkout d212962ef7682ee160bf38fa455475558f031759
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=openrisc SHELL=/bin/bash lib/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Link: https://lore.kernel.org/oe-kbuild-all/202304080811.nYP4KpPZ-lkp@intel.com/
>
> All warnings (new ones prefixed by >>):
>
> In file included from lib/fortify_kunit.c:28:
> lib/fortify_kunit.c: In function 'strnlen_test':
> >> lib/fortify_kunit.c:412:31: warning: 'strnlen' specified bound 33 exceeds source size 32 [-Wstringop-overread]
If we expect to validate the runtime behavior of fortify, but using
constants that the compiler can check for readability in this test,
then we might need to use the
_Pragma/__diag infrastructure from include/linux/compiler_types.h to
disable -Wstringop-overread; or disable it at the makefile level.
> 412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
> 584 | const typeof(left) __left = (left); \
> | ^~~~
> include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
> 776 | KUNIT_BINARY_INT_ASSERTION(test, \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
> 773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
> | ^~~~~~~~~~~~~~~~~~~
> lib/fortify_kunit.c:412:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
> 412 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> | ^~~~~~~~~~~~~~~
> lib/fortify_kunit.c:359:14: note: source object allocated here
> 359 | char buf[32];
> | ^~~
> lib/fortify_kunit.c:414:31: warning: 'strnlen' specified bound 34 exceeds source size 32 [-Wstringop-overread]
> 414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> include/kunit/test.h:584:38: note: in definition of macro 'KUNIT_BASE_BINARY_ASSERTION'
> 584 | const typeof(left) __left = (left); \
> | ^~~~
> include/kunit/test.h:776:9: note: in expansion of macro 'KUNIT_BINARY_INT_ASSERTION'
> 776 | KUNIT_BINARY_INT_ASSERTION(test, \
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~
> include/kunit/test.h:773:9: note: in expansion of macro 'KUNIT_EXPECT_EQ_MSG'
> 773 | KUNIT_EXPECT_EQ_MSG(test, left, right, NULL)
> | ^~~~~~~~~~~~~~~~~~~
> lib/fortify_kunit.c:414:9: note: in expansion of macro 'KUNIT_EXPECT_EQ'
> 414 | KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> | ^~~~~~~~~~~~~~~
> lib/fortify_kunit.c:359:14: note: source object allocated here
> 359 | char buf[32];
> | ^~~
>
>
> vim +/strnlen +412 lib/fortify_kunit.c
>
> 387
> 388 static void strnlen_test(struct kunit *test)
> 389 {
> 390 struct fortify_padding pad = { };
> 391 int i, end = sizeof(pad.buf) - 1;
> 392
> 393 /* Fill 31 bytes with valid characters. */
> 394 for (i = 0; i < sizeof(pad.buf) - 1; i++)
> 395 pad.buf[i] = i + '0';
> 396 /* Trailing bytes are still %NUL. */
> 397 KUNIT_EXPECT_EQ(test, pad.buf[end], '\0');
> 398 KUNIT_EXPECT_EQ(test, pad.bytes_after, 0);
> 399
> 400 /* String is terminated, so strnlen() is valid. */
> 401 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf)), end);
> 402 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> 403 /* A truncated strnlen() will be safe, too. */
> 404 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, sizeof(pad.buf) / 2),
> 405 sizeof(pad.buf) / 2);
> 406 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 0);
> 407
> 408 /* Make string unterminated, and recount. */
> 409 pad.buf[end] = 'A';
> 410 end = sizeof(pad.buf);
> 411 /* Reading beyond with strncpy() will fail. */
> > 412 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 1), end);
> 413 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 1);
> 414 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end + 2), end);
> 415 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 416
> 417 /* Early-truncated is safe still, though. */
> 418 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
> 419 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 420
> 421 end = sizeof(pad.buf) / 2;
> 422 KUNIT_EXPECT_EQ(test, strnlen(pad.buf, end), end);
> 423 KUNIT_EXPECT_EQ(test, fortify_read_overflows, 2);
> 424 }
> 425
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 2+ messages in thread