[PATCH v2 0/3] Fix FORTIFY=y UBSAN_LOCAL_BOUNDS=y

[PATCH v2 0/3] Fix FORTIFY=y UBSAN_LOCAL_BOUNDS=y

[Kees Cook]

With CONFIG_FORTIFY=y and CONFIG_UBSAN_LOCAL_BOUNDS=y enabled, we
observe a runtime panic while running Android's Compatibility Test
Suite's (CTS) android.hardware.input.cts.tests. This is stemming from a
strlen() call in hidinput_allocate().

__builtin_object_size(str, 0 or 1) has interesting behavior for C
strings when str is runtime dependent, and all possible values are known
at compile time; it evaluates to the maximum of those sizes. This causes
UBSAN_LOCAL_BOUNDS to insert faults for the smaller values, which we
trip at runtime.

Patch 1 is the actual fix, using a 0-index __builtin_constant_p() check
to short-circuit the runtime check.
Patch 2 is a KUnit test to validate this behavior going forward.
Patch 3 is is a cosmetic cleanup to use SIZE_MAX instead of (size_t)-1

-Kees

v2:
 - different solution
 - add KUnit test
 - expand scope of cosmetic cleanup
v1: https://lore.kernel.org/lkml/20220830205309.312864-1-ndesaulniers@google.com

Kees Cook (3):
  fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL
  fortify: Add KUnit test for FORTIFY_SOURCE internals
  fortify: Use SIZE_MAX instead of (size_t)-1

 MAINTAINERS                    |  1 +
 include/linux/fortify-string.h | 29 ++++++-------
 lib/Kconfig.debug              |  9 ++++
 lib/Makefile                   |  1 +
 lib/fortify_kunit.c            | 77 ++++++++++++++++++++++++++++++++++
 5 files changed, 103 insertions(+), 14 deletions(-)
 create mode 100644 lib/fortify_kunit.c

-- 
2.34.1

[PATCH v2 1/3] fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL

[Kees Cook]

With CONFIG_FORTIFY=y and CONFIG_UBSAN_LOCAL_BOUNDS=y enabled, we observe
a runtime panic while running Android's Compatibility Test Suite's (CTS)
android.hardware.input.cts.tests. This is stemming from a strlen()
call in hidinput_allocate().

__compiletime_strlen() is implemented in terms of __builtin_object_size(),
then does an array access to check for NUL-termination. A quirk of
__builtin_object_size() is that for strings whose values are runtime
dependent, __builtin_object_size(str, 1 or 0) returns the maximum size
of possible values when those sizes are determinable at compile time.
Example:

  static const char *v = "FOO BAR";
  static const char *y = "FOO BA";
  unsigned long x (int z) {
      // Returns 8, which is:
      // max(__builtin_object_size(v, 1), __builtin_object_size(y, 1))
      return __builtin_object_size(z ? v : y, 1);
  }

So when FORTIFY_SOURCE is enabled, the current implementation of
__compiletime_strlen() will try to access beyond the end of y at runtime
using the size of v. Mixed with UBSAN_LOCAL_BOUNDS we get a fault.

hidinput_allocate() has a local C string whose value is control flow
dependent on a switch statement, so __builtin_object_size(str, 1)
evaluates to the maximum string length, making all other cases fault on
the last character check. hidinput_allocate() could be cleaned up to
avoid runtime calls to strlen() since the local variable can only have
literal values, so there's no benefit to trying to fortify the strlen
call site there.

Perform a __builtin_constant_p() check against index 0 earlier in the
macro to filter out the control-flow-dependant case. Add a KUnit test
for checking the expected behavioral characteristics of FORTIFY_SOURCE
internals.

Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Tom Rix <trix@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Cc: David Gow <davidgow@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Sander Vanheule <sander@svanheule.net>
Cc: linux-hardening@vger.kernel.org
Cc: llvm@lists.linux.dev
Co-developed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index eed2119b23c5..07d5d1921eff 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -19,7 +19,8 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
 	unsigned char *__p = (unsigned char *)(p);		\
 	size_t __ret = (size_t)-1;				\
 	size_t __p_size = __builtin_object_size(p, 1);		\
-	if (__p_size != (size_t)-1) {				\
+	if (__p_size != (size_t)-1 &&				\
+	    __builtin_constant_p(*__p)) {			\
 		size_t __p_len = __p_size - 1;			\
 		if (__builtin_constant_p(__p[__p_len]) &&	\
 		    __p[__p_len] == '\0')			\
-- 
2.34.1

Re: [PATCH v2 1/3] fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL

[Nick Desaulniers]

On Fri, Sep 2, 2022 at 1:43 PM Kees Cook <keescook@chromium.org> wrote:
>
> Co-developed-by: Nick Desaulniers <ndesaulniers@google.com>

That's overly generous of you!

Anyways, the disassembly LGTM and the bot also came back green.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Android Treehugger Robot
Link: https://android-review.googlesource.com/c/kernel/common/+/2206839

Another thought, Nikita suggested that you could also compare mode 1 vs mode 3:
https://github.com/llvm/llvm-project/issues/57510#issuecomment-1235126343

That said, since mode 3 returns 0 for "unknown" I'd imagine that
wouldn't be pretty since it wouldn't be a direct comparison against
__p_size.

Thanks again for the patch!

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index eed2119b23c5..07d5d1921eff 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -19,7 +19,8 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
>         unsigned char *__p = (unsigned char *)(p);              \
>         size_t __ret = (size_t)-1;                              \
>         size_t __p_size = __builtin_object_size(p, 1);          \
> -       if (__p_size != (size_t)-1) {                           \
> +       if (__p_size != (size_t)-1 &&                           \
> +           __builtin_constant_p(*__p)) {                       \
>                 size_t __p_len = __p_size - 1;                  \
>                 if (__builtin_constant_p(__p[__p_len]) &&       \
>                     __p[__p_len] == '\0')                       \
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 1/3] fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL

[Kees Cook]

On Tue, Sep 06, 2022 at 07:36:46PM -0700, Nick Desaulniers wrote:
> On Fri, Sep 2, 2022 at 1:43 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Co-developed-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> That's overly generous of you!

Well, it was a lot of work to track down, and you wrote it up that way,
I just moved things around a little bit. :)

> Anyways, the disassembly LGTM and the bot also came back green.
> 
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Android Treehugger Robot
> Link: https://android-review.googlesource.com/c/kernel/common/+/2206839

Thank you!

> Another thought, Nikita suggested that you could also compare mode 1 vs mode 3:
> https://github.com/llvm/llvm-project/issues/57510#issuecomment-1235126343

Yeah, it could work (I tried this as well), but I think the better
approach is checking index 0.

> That said, since mode 3 returns 0 for "unknown" I'd imagine that
> wouldn't be pretty since it wouldn't be a direct comparison against
> __p_size.

Yeah -- it is a little weird. I might come back to this if we get more
glitches like this in the future.

-- 
Kees Cook

[PATCH v2 2/3] fortify: Add KUnit test for FORTIFY_SOURCE internals

[Kees Cook]

Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
characteristics of FORTIFY_SOURCE internals.

Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Tom Rix <trix@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
Cc: David Gow <davidgow@google.com>
Cc: Yury Norov <yury.norov@gmail.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Sander Vanheule <sander@svanheule.net>
Cc: linux-hardening@vger.kernel.org
Cc: llvm@lists.linux.dev
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 MAINTAINERS         |  1 +
 lib/Kconfig.debug   |  9 ++++++
 lib/Makefile        |  1 +
 lib/fortify_kunit.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 88 insertions(+)
 create mode 100644 lib/fortify_kunit.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 9d7f64dc0efe..640115472199 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8002,6 +8002,7 @@ L:	linux-hardening@vger.kernel.org
 S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
 F:	include/linux/fortify-string.h
+F:	lib/fortify_kunit.c
 F:	lib/test_fortify/*
 F:	scripts/test_fortify.sh
 K:	\b__NO_FORTIFY\b
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 36455953d306..1f267c0ddffd 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2542,6 +2542,15 @@ config STACKINIT_KUNIT_TEST
 	  CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
 	  or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
 
+config FORTIFY_KUNIT_TEST
+	tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
+	depends on KUNIT && FORTIFY_SOURCE
+	default KUNIT_ALL_TESTS
+	help
+	  Builds unit tests for checking internals of FORTIFY_SOURCE as used
+	  by the str*() and mem*() family of functions. For testing runtime
+	  traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
+
 config TEST_UDELAY
 	tristate "udelay test driver"
 	help
diff --git a/lib/Makefile b/lib/Makefile
index f545140ed9e7..4ee1ceae945a 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -381,6 +381,7 @@ obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
 obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
 CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
 obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
+obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 
 obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
 
diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
new file mode 100644
index 000000000000..4d7930b65107
--- /dev/null
+++ b/lib/fortify_kunit.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Runtime test cases for CONFIG_FORTIFY_SOURCE that aren't expected to
+ * Oops th kernel on success. (For those, see drivers/misc/lkdtm/fortify.c)
+ *
+ * For corner cases with UBSAN, try testing with:
+ *
+ * ./tools/testing/kunit/kunit.py run --arch=x86_64 \
+ *	--kconfig_add CONFIG_FORTIFY_SOURCE=y \
+ *	--kconfig_add CONFIG_UBSAN=y \
+ *	--kconfig_add CONFIG_UBSAN_TRAP=y \
+ *	--kconfig_add CONFIG_UBSAN_BOUNDS=y \
+ *	--kconfig_add CONFIG_UBSAN_LOCAL_BOUNDS=y \
+ *	--make_options LLVM=1 fortify
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <kunit/test.h>
+#include <linux/string.h>
+#include <linux/init.h>
+
+static const char array_of_10[] = "this is 10";
+static const char *ptr_of_11 = "this is 11!";
+static char array_unknown[] = "compiler thinks I might change";
+
+static void known_sizes_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
+	KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_of_10), 10);
+	KUNIT_EXPECT_EQ(test, __compiletime_strlen(ptr_of_11), 11);
+
+	KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_unknown), SIZE_MAX);
+	/* Externally defined and dynamically sized string pointer: */
+	KUNIT_EXPECT_EQ(test, __compiletime_strlen(saved_command_line), SIZE_MAX);
+}
+
+/* This is volatile so the optimizer can't perform DCE below. */
+static volatile int pick;
+
+/* Not inline to keep optimizer from figuring out which string we want. */
+static noinline size_t want_minus_one(int pick)
+{
+	const char *str;
+
+	switch (pick) {
+	case 1:
+		str = "4444";
+		break;
+	case 2:
+		str = "333";
+		break;
+	default:
+		str = "1";
+		break;
+	}
+	return __compiletime_strlen(str);
+}
+
+static void control_flow_split_test(struct kunit *test)
+{
+	KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX);
+}
+
+static struct kunit_case fortify_test_cases[] = {
+	KUNIT_CASE(known_sizes_test),
+	KUNIT_CASE(control_flow_split_test),
+	{}
+};
+
+static struct kunit_suite fortify_test_suite = {
+	.name = "fortify",
+	.test_cases = fortify_test_cases,
+};
+
+kunit_test_suite(fortify_test_suite);
+
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH v2 2/3] fortify: Add KUnit test for FORTIFY_SOURCE internals

[David Gow]

On Sat, Sep 3, 2022 at 4:43 AM Kees Cook <keescook@chromium.org> wrote:
>
> Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
> characteristics of FORTIFY_SOURCE internals.
>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Tom Rix <trix@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> Cc: David Gow <davidgow@google.com>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Sander Vanheule <sander@svanheule.net>
> Cc: linux-hardening@vger.kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---

Overall, this looks good. It's a bit of a shame FORTIFY_SOURCE doesn't
work under UML, but I tested it on everything else I had to hand and
it looked good.

One tiny typo in a comment below, but otherwise this is:

Reviewed-by: David Gow <davidgow@google.com>

Cheers,
-- David

>  MAINTAINERS         |  1 +
>  lib/Kconfig.debug   |  9 ++++++
>  lib/Makefile        |  1 +
>  lib/fortify_kunit.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+)
>  create mode 100644 lib/fortify_kunit.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9d7f64dc0efe..640115472199 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8002,6 +8002,7 @@ L:        linux-hardening@vger.kernel.org
>  S:     Supported
>  T:     git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
>  F:     include/linux/fortify-string.h
> +F:     lib/fortify_kunit.c
>  F:     lib/test_fortify/*
>  F:     scripts/test_fortify.sh
>  K:     \b__NO_FORTIFY\b
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 36455953d306..1f267c0ddffd 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2542,6 +2542,15 @@ config STACKINIT_KUNIT_TEST
>           CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
>           or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>
> +config FORTIFY_KUNIT_TEST
> +       tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
> +       depends on KUNIT && FORTIFY_SOURCE
> +       default KUNIT_ALL_TESTS
> +       help
> +         Builds unit tests for checking internals of FORTIFY_SOURCE as used
> +         by the str*() and mem*() family of functions. For testing runtime
> +         traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
> +
>  config TEST_UDELAY
>         tristate "udelay test driver"
>         help
> diff --git a/lib/Makefile b/lib/Makefile
> index f545140ed9e7..4ee1ceae945a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -381,6 +381,7 @@ obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
>  obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
>  CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
>  obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
> +obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
>
>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>
> diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> new file mode 100644
> index 000000000000..4d7930b65107
> --- /dev/null
> +++ b/lib/fortify_kunit.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Runtime test cases for CONFIG_FORTIFY_SOURCE that aren't expected to
> + * Oops th kernel on success. (For those, see drivers/misc/lkdtm/fortify.c)

Nit: Oops _the_ kernel

> + *
> + * For corner cases with UBSAN, try testing with:
> + *
> + * ./tools/testing/kunit/kunit.py run --arch=x86_64 \
> + *     --kconfig_add CONFIG_FORTIFY_SOURCE=y \
> + *     --kconfig_add CONFIG_UBSAN=y \
> + *     --kconfig_add CONFIG_UBSAN_TRAP=y \
> + *     --kconfig_add CONFIG_UBSAN_BOUNDS=y \
> + *     --kconfig_add CONFIG_UBSAN_LOCAL_BOUNDS=y \
> + *     --make_options LLVM=1 fortify
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <kunit/test.h>
> +#include <linux/string.h>
> +#include <linux/init.h>
> +
> +static const char array_of_10[] = "this is 10";
> +static const char *ptr_of_11 = "this is 11!";
> +static char array_unknown[] = "compiler thinks I might change";
> +
> +static void known_sizes_test(struct kunit *test)
> +{
> +       KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
> +       KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_of_10), 10);
> +       KUNIT_EXPECT_EQ(test, __compiletime_strlen(ptr_of_11), 11);
> +
> +       KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_unknown), SIZE_MAX);
> +       /* Externally defined and dynamically sized string pointer: */
> +       KUNIT_EXPECT_EQ(test, __compiletime_strlen(saved_command_line), SIZE_MAX);
> +}
> +
> +/* This is volatile so the optimizer can't perform DCE below. */
> +static volatile int pick;
> +
> +/* Not inline to keep optimizer from figuring out which string we want. */
> +static noinline size_t want_minus_one(int pick)
> +{
> +       const char *str;
> +
> +       switch (pick) {
> +       case 1:
> +               str = "4444";
> +               break;
> +       case 2:
> +               str = "333";
> +               break;
> +       default:
> +               str = "1";
> +               break;
> +       }
> +       return __compiletime_strlen(str);
> +}
> +
> +static void control_flow_split_test(struct kunit *test)
> +{
> +       KUNIT_EXPECT_EQ(test, want_minus_one(pick), SIZE_MAX);
> +}
> +
> +static struct kunit_case fortify_test_cases[] = {
> +       KUNIT_CASE(known_sizes_test),
> +       KUNIT_CASE(control_flow_split_test),
> +       {}
> +};
> +
> +static struct kunit_suite fortify_test_suite = {
> +       .name = "fortify",
> +       .test_cases = fortify_test_cases,
> +};
> +
> +kunit_test_suite(fortify_test_suite);
> +
> +MODULE_LICENSE("GPL");
> --
> 2.34.1
>

Re: [PATCH v2 2/3] fortify: Add KUnit test for FORTIFY_SOURCE internals

[Kees Cook]

On Sat, Sep 03, 2022 at 10:59:24AM +0800, David Gow wrote:
> On Sat, Sep 3, 2022 at 4:43 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
> > characteristics of FORTIFY_SOURCE internals.
> > [...]
> 
> Overall, this looks good. It's a bit of a shame FORTIFY_SOURCE doesn't
> work under UML, but I tested it on everything else I had to hand and
> it looked good.

It looks like this was never picked up:
https://lore.kernel.org/lkml/20220210003224.773957-1-keescook@chromium.org/

I suppose I could take it via the kernel hardening tree?

> One tiny typo in a comment below, but otherwise this is:
> 
> Reviewed-by: David Gow <davidgow@google.com>
> 
> [...]
> > +/*
> > + * Runtime test cases for CONFIG_FORTIFY_SOURCE that aren't expected to
> > + * Oops th kernel on success. (For those, see drivers/misc/lkdtm/fortify.c)
> 
> Nit: Oops _the_ kernel

Thanks! I'll get that updated. :)

-- 
Kees Cook

Re: [PATCH v2 2/3] fortify: Add KUnit test for FORTIFY_SOURCE internals

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Sat, Sep 3, 2022 at 1:17 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Sep 03, 2022 at 10:59:24AM +0800, David Gow wrote:
> > On Sat, Sep 3, 2022 at 4:43 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
> > > characteristics of FORTIFY_SOURCE internals.
> > > [...]
> >
> > Overall, this looks good. It's a bit of a shame FORTIFY_SOURCE doesn't
> > work under UML, but I tested it on everything else I had to hand and
> > it looked good.
>
> It looks like this was never picked up:
> https://lore.kernel.org/lkml/20220210003224.773957-1-keescook@chromium.org/

Ah: I'd forgotten about that. Can confirm this test works under UML
with that patch applied.

Cheers,
-- David

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4003 bytes --]

Re: [PATCH v2 2/3] fortify: Add KUnit test for FORTIFY_SOURCE internals

[Nathan Chancellor]

Hi Kees,

On Fri, Sep 02, 2022 at 01:43:50PM -0700, Kees Cook wrote:
> Add lib/fortify_kunit.c KUnit test for checking the expected behavioral
> characteristics of FORTIFY_SOURCE internals.
> 
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Tom Rix <trix@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: "Steven Rostedt (Google)" <rostedt@goodmis.org>
> Cc: David Gow <davidgow@google.com>
> Cc: Yury Norov <yury.norov@gmail.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Sander Vanheule <sander@svanheule.net>
> Cc: linux-hardening@vger.kernel.org
> Cc: llvm@lists.linux.dev
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  MAINTAINERS         |  1 +
>  lib/Kconfig.debug   |  9 ++++++
>  lib/Makefile        |  1 +
>  lib/fortify_kunit.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 88 insertions(+)
>  create mode 100644 lib/fortify_kunit.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 9d7f64dc0efe..640115472199 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8002,6 +8002,7 @@ L:	linux-hardening@vger.kernel.org
>  S:	Supported
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/hardening
>  F:	include/linux/fortify-string.h
> +F:	lib/fortify_kunit.c
>  F:	lib/test_fortify/*
>  F:	scripts/test_fortify.sh
>  K:	\b__NO_FORTIFY\b
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 36455953d306..1f267c0ddffd 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2542,6 +2542,15 @@ config STACKINIT_KUNIT_TEST
>  	  CONFIG_GCC_PLUGIN_STRUCTLEAK, CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF,
>  	  or CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
>  
> +config FORTIFY_KUNIT_TEST
> +	tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
> +	depends on KUNIT && FORTIFY_SOURCE
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Builds unit tests for checking internals of FORTIFY_SOURCE as used
> +	  by the str*() and mem*() family of functions. For testing runtime
> +	  traps of FORTIFY_SOURCE, see LKDTM's "FORTIFY_*" tests.
> +
>  config TEST_UDELAY
>  	tristate "udelay test driver"
>  	help
> diff --git a/lib/Makefile b/lib/Makefile
> index f545140ed9e7..4ee1ceae945a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -381,6 +381,7 @@ obj-$(CONFIG_IS_SIGNED_TYPE_KUNIT_TEST) += is_signed_type_kunit.o
>  obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
>  CFLAGS_stackinit_kunit.o += $(call cc-disable-warning, switch-unreachable)
>  obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
> +obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
>  
>  obj-$(CONFIG_GENERIC_LIB_DEVMEM_IS_ALLOWED) += devmem_is_allowed.o
>  
> diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c
> new file mode 100644
> index 000000000000..4d7930b65107
> --- /dev/null
> +++ b/lib/fortify_kunit.c
> @@ -0,0 +1,77 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Runtime test cases for CONFIG_FORTIFY_SOURCE that aren't expected to
> + * Oops th kernel on success. (For those, see drivers/misc/lkdtm/fortify.c)
> + *
> + * For corner cases with UBSAN, try testing with:
> + *
> + * ./tools/testing/kunit/kunit.py run --arch=x86_64 \
> + *	--kconfig_add CONFIG_FORTIFY_SOURCE=y \
> + *	--kconfig_add CONFIG_UBSAN=y \
> + *	--kconfig_add CONFIG_UBSAN_TRAP=y \
> + *	--kconfig_add CONFIG_UBSAN_BOUNDS=y \
> + *	--kconfig_add CONFIG_UBSAN_LOCAL_BOUNDS=y \
> + *	--make_options LLVM=1 fortify
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <kunit/test.h>
> +#include <linux/string.h>
> +#include <linux/init.h>
> +
> +static const char array_of_10[] = "this is 10";
> +static const char *ptr_of_11 = "this is 11!";
> +static char array_unknown[] = "compiler thinks I might change";
> +
> +static void known_sizes_test(struct kunit *test)
> +{
> +	KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
> +	KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_of_10), 10);
> +	KUNIT_EXPECT_EQ(test, __compiletime_strlen(ptr_of_11), 11);
> +
> +	KUNIT_EXPECT_EQ(test, __compiletime_strlen(array_unknown), SIZE_MAX);
> +	/* Externally defined and dynamically sized string pointer: */
> +	KUNIT_EXPECT_EQ(test, __compiletime_strlen(saved_command_line), SIZE_MAX);
> +}

I noticed that my allmodconfig + ThinLTO builds broke with this change:

  $ make -skj"$(nproc)" ARCH=arm64 KCONFIG_ALLCONFIG=<(printf "CONFIG_%s=n\nCONFIG_%s=n\nCONFIG_%s=y\nCONFIG_%s=n\n" GCOV_KERNEL KASAN LTO_CLANG_THIN WERROR) LLVM=1 mrproper allmodconfig all
  ...
  ERROR: modpost: "saved_command_line" [lib/fortify_kunit.ko] undefined!
  ...

saved_command_line is not exported to modules. This appears to be
related to the fact that CONFIG_KCSAN gets enabled due to the way
KCONFIG_ALLCONFIG works, as the issue does not show up in our CI, which
uses merge_config.sh.

A smaller reproducer on top of defconfig:

  $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 defconfig

  $ scripts/config \
  -d LTO_NONE \
  -e EXPERT \
  -e FORTIFY_SOURCE \
  -e KCSAN \
  -e LTO_CLANG_THIN \
  -m FORTIFY_KUNIT_TEST \
  -m KUNIT

  $ make -skj"$(nproc)" ARCH=arm64 LLVM=1 olddefconfig all
  ...
  ERROR: modpost: "saved_command_line" [lib/fortify_kunit.ko] undefined!
  ...

I don't see this without LTO. Is this a compiler problem?

Small note, Marco's patch is needed to avoid a separate linking error
with ToT LLVM:

https://lore.kernel.org/20220912094541.929856-1-elver@google.com/

Cheers,
Nathan

[PATCH v2 3/3] fortify: Use SIZE_MAX instead of (size_t)-1

[Kees Cook]

Clean up uses of "(size_t)-1" in favor of SIZE_MAX.

Cc: linux-hardening@vger.kernel.org
Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/fortify-string.h | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index 07d5d1921eff..8f2b6b1cb848 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -17,9 +17,9 @@ void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("
 #define __compiletime_strlen(p)					\
 ({								\
 	unsigned char *__p = (unsigned char *)(p);		\
-	size_t __ret = (size_t)-1;				\
+	size_t __ret = SIZE_MAX;				\
 	size_t __p_size = __builtin_object_size(p, 1);		\
-	if (__p_size != (size_t)-1 &&				\
+	if (__p_size != SIZE_MAX &&				\
 	    __builtin_constant_p(*__p)) {			\
 		size_t __p_len = __p_size - 1;			\
 		if (__builtin_constant_p(__p[__p_len]) &&	\
@@ -125,7 +125,7 @@ char *strcat(char * const POS p, const char *q)
 {
 	size_t p_size = __builtin_object_size(p, 1);
 
-	if (p_size == (size_t)-1)
+	if (p_size == SIZE_MAX)
 		return __underlying_strcat(p, q);
 	if (strlcat(p, q, p_size) >= p_size)
 		fortify_panic(__func__);
@@ -140,7 +140,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
 	size_t ret;
 
 	/* We can take compile-time actions when maxlen is const. */
-	if (__builtin_constant_p(maxlen) && p_len != (size_t)-1) {
+	if (__builtin_constant_p(maxlen) && p_len != SIZE_MAX) {
 		/* If p is const, we can use its compile-time-known len. */
 		if (maxlen >= p_size)
 			return p_len;
@@ -168,7 +168,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
 	size_t p_size = __builtin_object_size(p, 1);
 
 	/* Give up if we don't know how large p is. */
-	if (p_size == (size_t)-1)
+	if (p_size == SIZE_MAX)
 		return __underlying_strlen(p);
 	ret = strnlen(p, p_size);
 	if (p_size <= ret)
@@ -185,7 +185,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
 	size_t q_len;	/* Full count of source string length. */
 	size_t len;	/* Count of characters going into destination. */
 
-	if (p_size == (size_t)-1 && q_size == (size_t)-1)
+	if (p_size == SIZE_MAX && q_size == SIZE_MAX)
 		return __real_strlcpy(p, q, size);
 	q_len = strlen(q);
 	len = (q_len >= size) ? size - 1 : q_len;
@@ -213,7 +213,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
 	size_t q_size = __builtin_object_size(q, 1);
 
 	/* If we cannot get size of p and q default to call strscpy. */
-	if (p_size == (size_t) -1 && q_size == (size_t) -1)
+	if (p_size == SIZE_MAX && q_size == SIZE_MAX)
 		return __real_strscpy(p, q, size);
 
 	/*
@@ -258,7 +258,7 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
 	size_t p_size = __builtin_object_size(p, 1);
 	size_t q_size = __builtin_object_size(q, 1);
 
-	if (p_size == (size_t)-1 && q_size == (size_t)-1)
+	if (p_size == SIZE_MAX && q_size == SIZE_MAX)
 		return __underlying_strncat(p, q, count);
 	p_len = strlen(p);
 	copy_len = strnlen(q, count);
@@ -299,10 +299,10 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
 	/*
 	 * Always stop accesses beyond the struct that contains the
 	 * field, when the buffer's remaining size is known.
-	 * (The -1 test is to optimize away checks where the buffer
+	 * (The SIZE_MAX test is to optimize away checks where the buffer
 	 * lengths are unknown.)
 	 */
-	if (p_size != (size_t)(-1) && p_size < size)
+	if (p_size != SIZE_MAX && p_size < size)
 		fortify_panic("memset");
 }
 
@@ -393,11 +393,11 @@ __FORTIFY_INLINE void fortify_memcpy_chk(__kernel_size_t size,
 	/*
 	 * Always stop accesses beyond the struct that contains the
 	 * field, when the buffer's remaining size is known.
-	 * (The -1 test is to optimize away checks where the buffer
+	 * (The SIZE_MAX test is to optimize away checks where the buffer
 	 * lengths are unknown.)
 	 */
-	if ((p_size != (size_t)(-1) && p_size < size) ||
-	    (q_size != (size_t)(-1) && q_size < size))
+	if ((p_size != SIZE_MAX && p_size < size) ||
+	    (q_size != SIZE_MAX && q_size < size))
 		fortify_panic(func);
 }
 
@@ -496,7 +496,7 @@ char *strcpy(char * const POS p, const char * const POS q)
 	size_t size;
 
 	/* If neither buffer size is known, immediately give up. */
-	if (p_size == (size_t)-1 && q_size == (size_t)-1)
+	if (p_size == SIZE_MAX && q_size == SIZE_MAX)
 		return __underlying_strcpy(p, q);
 	size = strlen(q) + 1;
 	/* Compile-time check for const size overflow. */
-- 
2.34.1

Re: [PATCH v2 3/3] fortify: Use SIZE_MAX instead of (size_t)-1

[Kees Cook]

On Fri, Sep 02, 2022 at 01:43:51PM -0700, Kees Cook wrote:
> Clean up uses of "(size_t)-1" in favor of SIZE_MAX.
> 
> Cc: linux-hardening@vger.kernel.org
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/fortify-string.h | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)

*brown paper bag*

This needs:

#include <linux/limits.h>

I will fix it locally...

-- 
Kees Cook

Re: [PATCH v2 3/3] fortify: Use SIZE_MAX instead of (size_t)-1

[kernel test robot]

Hi Kees,

I love your patch! Yet something to improve:

[auto build test ERROR on akpm-mm/mm-everything]
[also build test ERROR on linus/master kees/for-next/pstore v6.0-rc3 next-20220901]
[cannot apply to kees/for-next/hardening]
[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/Fix-FORTIFY-y-UBSAN_LOCAL_BOUNDS-y/20220903-044622
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20220903/202209030625.wdoOhjAW-lkp@intel.com/config)
compiler: powerpc-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/6c991b3511a531e44e856fdf2b64020b70fd7b22
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Kees-Cook/Fix-FORTIFY-y-UBSAN_LOCAL_BOUNDS-y/20220903-044622
        git checkout 6c991b3511a531e44e856fdf2b64020b70fd7b22
        # 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=powerpc prepare

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
   scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
   In file included from include/linux/string.h:253,
                    from include/linux/uuid.h:12,
                    from include/linux/mod_devicetable.h:13,
                    from scripts/mod/devicetable-offsets.c:3:
   include/linux/fortify-string.h: In function 'strcat':
>> include/linux/fortify-string.h:98:23: error: 'SIZE_MAX' undeclared (first use in this function)
      98 |         if (p_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:6:1: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
       5 | #include <linux/const.h>
     +++ |+#include <stdint.h>
       6 | 
   include/linux/fortify-string.h:98:23: note: each undeclared identifier is reported only once for each function it appears in
      98 |         if (p_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h: In function 'strnlen':
   include/linux/fortify-string.h:20:24: error: 'SIZE_MAX' undeclared (first use in this function)
      20 |         size_t __ret = SIZE_MAX;                                \
         |                        ^~~~~~~~
   include/linux/fortify-string.h:109:24: note: in expansion of macro '__compiletime_strlen'
     109 |         size_t p_len = __compiletime_strlen(p);
         |                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h:20:24: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
      20 |         size_t __ret = SIZE_MAX;                                \
         |                        ^~~~~~~~
   include/linux/fortify-string.h:109:24: note: in expansion of macro '__compiletime_strlen'
     109 |         size_t p_len = __compiletime_strlen(p);
         |                        ^~~~~~~~~~~~~~~~~~~~
   include/linux/fortify-string.h: In function '__fortify_strlen':
   include/linux/fortify-string.h:141:23: error: 'SIZE_MAX' undeclared (first use in this function)
     141 |         if (p_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:141:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'strlcpy':
   include/linux/fortify-string.h:158:23: error: 'SIZE_MAX' undeclared (first use in this function)
     158 |         if (p_size == SIZE_MAX && q_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:158:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'strscpy':
   include/linux/fortify-string.h:186:23: error: 'SIZE_MAX' undeclared (first use in this function)
     186 |         if (p_size == SIZE_MAX && q_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:186:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'strncat':
   include/linux/fortify-string.h:231:23: error: 'SIZE_MAX' undeclared (first use in this function)
     231 |         if (p_size == SIZE_MAX && q_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:231:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'fortify_memset_chk':
   include/linux/fortify-string.h:275:23: error: 'SIZE_MAX' undeclared (first use in this function)
     275 |         if (p_size != SIZE_MAX && p_size < size)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:275:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'fortify_memcpy_chk':
   include/linux/fortify-string.h:369:24: error: 'SIZE_MAX' undeclared (first use in this function)
     369 |         if ((p_size != SIZE_MAX && p_size < size) ||
         |                        ^~~~~~~~
   include/linux/fortify-string.h:369:24: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   include/linux/fortify-string.h: In function 'strcpy':
   include/linux/fortify-string.h:469:23: error: 'SIZE_MAX' undeclared (first use in this function)
     469 |         if (p_size == SIZE_MAX && q_size == SIZE_MAX)
         |                       ^~~~~~~~
   include/linux/fortify-string.h:469:23: note: 'SIZE_MAX' is defined in header '<stdint.h>'; did you forget to '#include <stdint.h>'?
   make[2]: *** [scripts/Makefile.build:117: scripts/mod/devicetable-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1204: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:222: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +/SIZE_MAX +98 include/linux/fortify-string.h

    92	
    93	__FORTIFY_INLINE __diagnose_as(__builtin_strcat, 1, 2)
    94	char *strcat(char * const POS p, const char *q)
    95	{
    96		size_t p_size = __builtin_object_size(p, 1);
    97	
  > 98		if (p_size == SIZE_MAX)
    99			return __underlying_strcat(p, q);
   100		if (strlcat(p, q, p_size) >= p_size)
   101			fortify_panic(__func__);
   102		return p;
   103	}
   104	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH v2 0/3] Fix FORTIFY=y UBSAN_LOCAL_BOUNDS=y

[Nick Desaulniers]

On Fri, Sep 2, 2022 at 1:43 PM Kees Cook <keescook@chromium.org> wrote:
>
> With CONFIG_FORTIFY=y and CONFIG_UBSAN_LOCAL_BOUNDS=y enabled, we
> observe a runtime panic while running Android's Compatibility Test
> Suite's (CTS) android.hardware.input.cts.tests. This is stemming from a
> strlen() call in hidinput_allocate().
>
> __builtin_object_size(str, 0 or 1) has interesting behavior for C
> strings when str is runtime dependent, and all possible values are known
> at compile time; it evaluates to the maximum of those sizes. This causes
> UBSAN_LOCAL_BOUNDS to insert faults for the smaller values, which we
> trip at runtime.
>
> Patch 1 is the actual fix, using a 0-index __builtin_constant_p() check
> to short-circuit the runtime check.
> Patch 2 is a KUnit test to validate this behavior going forward.
> Patch 3 is is a cosmetic cleanup to use SIZE_MAX instead of (size_t)-1

Thanks,
Testing out patch 1/3 against Android's CTS:
https://android-review.googlesource.com/c/kernel/common/+/2206839,
will give formal signoffs/review after a completed test run.

>
> -Kees
>
> v2:
>  - different solution
>  - add KUnit test
>  - expand scope of cosmetic cleanup
> v1: https://lore.kernel.org/lkml/20220830205309.312864-1-ndesaulniers@google.com
>
> Kees Cook (3):
>   fortify: Fix __compiletime_strlen() under UBSAN_BOUNDS_LOCAL
>   fortify: Add KUnit test for FORTIFY_SOURCE internals
>   fortify: Use SIZE_MAX instead of (size_t)-1
>
>  MAINTAINERS                    |  1 +
>  include/linux/fortify-string.h | 29 ++++++-------
>  lib/Kconfig.debug              |  9 ++++
>  lib/Makefile                   |  1 +
>  lib/fortify_kunit.c            | 77 ++++++++++++++++++++++++++++++++++
>  5 files changed, 103 insertions(+), 14 deletions(-)
>  create mode 100644 lib/fortify_kunit.c
>
> --
> 2.34.1
>


-- 
Thanks,
~Nick Desaulniers