Merge tag 'hardening-v6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux

Pull hardening updates from Kees Cook:
 "There are three areas of note:

  A bunch of strlcpy()->strscpy() conversions ended up living in my tree
  since they were either Acked by maintainers for me to carry, or got
  ignored for multiple weeks (and were trivial changes).

  The compiler option '-fstrict-flex-arrays=3' has been enabled
  globally, and has been in -next for the entire devel cycle. This
  changes compiler diagnostics (though mainly just -Warray-bounds which
  is disabled) and potential UBSAN_BOUNDS and FORTIFY _warning_
  coverage. In other words, there are no new restrictions, just
  potentially new warnings. Any new FORTIFY warnings we've seen have
  been fixed (usually in their respective subsystem trees). For more
  details, see commit df8fc4e934.

  The under-development compiler attribute __counted_by has been added
  so that we can start annotating flexible array members with their
  associated structure member that tracks the count of flexible array
  elements at run-time. It is possible (likely?) that the exact syntax
  of the attribute will change before it is finalized, but GCC and Clang
  are working together to sort it out. Any changes can be made to the
  macro while we continue to add annotations.

  As an example of that last case, I have a treewide commit waiting with
  such annotations found via Coccinelle:

    https://git.kernel.org/linus/adc5b3cb48a049563dc673f348eab7b6beba8a9b

  Also see commit dd06e72e68 for more details.

  Summary:

   - Fix KMSAN vs FORTIFY in strlcpy/strlcat (Alexander Potapenko)

   - Convert strreplace() to return string start (Andy Shevchenko)

   - Flexible array conversions (Arnd Bergmann, Wyes Karny, Kees Cook)

   - Add missing function prototypes seen with W=1 (Arnd Bergmann)

   - Fix strscpy() kerndoc typo (Arne Welzel)

   - Replace strlcpy() with strscpy() across many subsystems which were
     either Acked by respective maintainers or were trivial changes that
     went ignored for multiple weeks (Azeem Shaikh)

   - Remove unneeded cc-option test for UBSAN_TRAP (Nick Desaulniers)

   - Add KUnit tests for strcat()-family

   - Enable KUnit tests of FORTIFY wrappers under UML

   - Add more complete FORTIFY protections for strlcat()

   - Add missed disabling of FORTIFY for all arch purgatories.

   - Enable -fstrict-flex-arrays=3 globally

   - Tightening UBSAN_BOUNDS when using GCC

   - Improve checkpatch to check for strcpy, strncpy, and fake flex
     arrays

   - Improve use of const variables in FORTIFY

   - Add requested struct_size_t() helper for types not pointers

   - Add __counted_by macro for annotating flexible array size members"

* tag 'hardening-v6.5-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux: (54 commits)
  netfilter: ipset: Replace strlcpy with strscpy
  uml: Replace strlcpy with strscpy
  um: Use HOST_DIR for mrproper
  kallsyms: Replace all non-returning strlcpy with strscpy
  sh: Replace all non-returning strlcpy with strscpy
  of/flattree: Replace all non-returning strlcpy with strscpy
  sparc64: Replace all non-returning strlcpy with strscpy
  Hexagon: Replace all non-returning strlcpy with strscpy
  kobject: Use return value of strreplace()
  lib/string_helpers: Change returned value of the strreplace()
  jbd2: Avoid printing outside the boundary of the buffer
  checkpatch: Check for 0-length and 1-element arrays
  riscv/purgatory: Do not use fortified string functions
  s390/purgatory: Do not use fortified string functions
  x86/purgatory: Do not use fortified string functions
  acpi: Replace struct acpi_table_slit 1-element array with flex-array
  clocksource: Replace all non-returning strlcpy with strscpy
  string: use __builtin_memcpy() in strlcpy/strlcat
  staging: most: Replace all non-returning strlcpy with strscpy
  drm/i2c: tda998x: Replace all non-returning strlcpy with strscpy
  ...

lib/Kconfig.debug

@@ -2675,7 +2675,7 @@ config STACKINIT_KUNIT_TEST
 
 config FORTIFY_KUNIT_TEST
 	tristate "Test fortified str*() and mem*() function internals at runtime" if !KUNIT_ALL_TESTS
-	depends on KUNIT && FORTIFY_SOURCE
+	depends on KUNIT
 	default KUNIT_ALL_TESTS
 	help
 	  Builds unit tests for checking internals of FORTIFY_SOURCE as used
@@ -2692,6 +2692,11 @@ config HW_BREAKPOINT_KUNIT_TEST
 
 	  If unsure, say N.
 
+config STRCAT_KUNIT_TEST
+	tristate "Test strcat() family of functions at runtime" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+
 config STRSCPY_KUNIT_TEST
 	tristate "Test strscpy*() family of functions at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT

lib/Kconfig.ubsan

@@ -15,7 +15,6 @@ if UBSAN
 config UBSAN_TRAP
 	bool "On Sanitizer warnings, abort the running kernel code"
 	depends on !COMPILE_TEST
-	depends on $(cc-option, -fsanitize-undefined-trap-on-error)
 	help
 	  Building kernels with Sanitizer features enabled tends to grow
 	  the kernel size by around 5%, due to adding all the debugging
@@ -27,16 +26,29 @@ config UBSAN_TRAP
 	  the system. For some system builders this is an acceptable
 	  trade-off.
 
-config CC_HAS_UBSAN_BOUNDS
-	def_bool $(cc-option,-fsanitize=bounds)
+config CC_HAS_UBSAN_BOUNDS_STRICT
+	def_bool $(cc-option,-fsanitize=bounds-strict)
+	help
+	  The -fsanitize=bounds-strict option is only available on GCC,
+	  but uses the more strict handling of arrays that includes knowledge
+	  of flexible arrays, which is comparable to Clang's regular
+	  -fsanitize=bounds.
 
 config CC_HAS_UBSAN_ARRAY_BOUNDS
 	def_bool $(cc-option,-fsanitize=array-bounds)
+	help
+	  Under Clang, the -fsanitize=bounds option is actually composed
+	  of two more specific options, -fsanitize=array-bounds and
+	  -fsanitize=local-bounds. However, -fsanitize=local-bounds can
+	  only be used when trap mode is enabled. (See also the help for
+	  CONFIG_LOCAL_BOUNDS.) Explicitly check for -fsanitize=array-bounds
+	  so that we can build up the options needed for UBSAN_BOUNDS
+	  with or without UBSAN_TRAP.
 
 config UBSAN_BOUNDS
 	bool "Perform array index bounds checking"
 	default UBSAN
-	depends on CC_HAS_UBSAN_ARRAY_BOUNDS || CC_HAS_UBSAN_BOUNDS
+	depends on CC_HAS_UBSAN_ARRAY_BOUNDS || CC_HAS_UBSAN_BOUNDS_STRICT
 	help
 	  This option enables detection of directly indexed out of bounds
 	  array accesses, where the array size is known at compile time.
@@ -44,33 +56,26 @@ config UBSAN_BOUNDS
 	  to the {str,mem}*cpy() family of functions (that is addressed
 	  by CONFIG_FORTIFY_SOURCE).
 
-config UBSAN_ONLY_BOUNDS
-	def_bool CC_HAS_UBSAN_BOUNDS && !CC_HAS_UBSAN_ARRAY_BOUNDS
-	depends on UBSAN_BOUNDS
+config UBSAN_BOUNDS_STRICT
+	def_bool UBSAN_BOUNDS && CC_HAS_UBSAN_BOUNDS_STRICT
 	help
-	  This is a weird case: Clang's -fsanitize=bounds includes
-	  -fsanitize=local-bounds, but it's trapping-only, so for
-	  Clang, we must use -fsanitize=array-bounds when we want
-	  traditional array bounds checking enabled. For GCC, we
-	  want -fsanitize=bounds.
+	  GCC's bounds sanitizer. This option is used to select the
+	  correct options in Makefile.ubsan.
 
 config UBSAN_ARRAY_BOUNDS
-	def_bool CC_HAS_UBSAN_ARRAY_BOUNDS
-	depends on UBSAN_BOUNDS
+	def_bool UBSAN_BOUNDS && CC_HAS_UBSAN_ARRAY_BOUNDS
+	help
+	  Clang's array bounds sanitizer. This option is used to select
+	  the correct options in Makefile.ubsan.
 
 config UBSAN_LOCAL_BOUNDS
-	bool "Perform array local bounds checking"
-	depends on UBSAN_TRAP
-	depends on $(cc-option,-fsanitize=local-bounds)
+	def_bool UBSAN_ARRAY_BOUNDS && UBSAN_TRAP
 	help
-	  This option enables -fsanitize=local-bounds which traps when an
-	  exception/error is detected. Therefore, it may only be enabled
-	  with CONFIG_UBSAN_TRAP.
-
-	  Enabling this option detects errors due to accesses through a
-	  pointer that is derived from an object of a statically-known size,
-	  where an added offset (which may not be known statically) is
-	  out-of-bounds.
+	  This option enables Clang's -fsanitize=local-bounds which traps
+	  when an access through a pointer that is derived from an object
+	  of a statically-known size, where an added offset (which may not
+	  be known statically) is out-of-bounds. Since this option is
+	  trap-only, it depends on CONFIG_UBSAN_TRAP.
 
 config UBSAN_SHIFT
 	bool "Perform checking for bit-shift overflows"

lib/Makefile

@@ -393,6 +393,7 @@ obj-$(CONFIG_STACKINIT_KUNIT_TEST) += stackinit_kunit.o
 CFLAGS_fortify_kunit.o += $(call cc-disable-warning, unsequenced)
 CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
 obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
+obj-$(CONFIG_STRCAT_KUNIT_TEST) += strcat_kunit.o
 obj-$(CONFIG_STRSCPY_KUNIT_TEST) += strscpy_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 

lib/fortify_kunit.c

@@ -25,6 +25,11 @@ 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";
 
+/* Handle being built without CONFIG_FORTIFY_SOURCE */
+#ifndef __compiletime_strlen
+# define __compiletime_strlen __builtin_strlen
+#endif
+
 static void known_sizes_test(struct kunit *test)
 {
 	KUNIT_EXPECT_EQ(test, __compiletime_strlen("88888888"), 8);
@@ -307,6 +312,14 @@ DEFINE_ALLOC_SIZE_TEST_PAIR(kvmalloc)
 } while (0)
 DEFINE_ALLOC_SIZE_TEST_PAIR(devm_kmalloc)
 
+static int fortify_test_init(struct kunit *test)
+{
+	if (!IS_ENABLED(CONFIG_FORTIFY_SOURCE))
+		kunit_skip(test, "Not built with CONFIG_FORTIFY_SOURCE=y");
+
+	return 0;
+}
+
 static struct kunit_case fortify_test_cases[] = {
 	KUNIT_CASE(known_sizes_test),
 	KUNIT_CASE(control_flow_split_test),
@@ -323,6 +336,7 @@ static struct kunit_case fortify_test_cases[] = {
 
 static struct kunit_suite fortify_test_suite = {
 	.name = "fortify",
+	.init = fortify_test_init,
 	.test_cases = fortify_test_cases,
 };
 

lib/kobject.c

@@ -281,8 +281,7 @@ int kobject_set_name_vargs(struct kobject *kobj, const char *fmt,
 		kfree_const(s);
 		if (!t)
 			return -ENOMEM;
-		strreplace(t, '/', '!');
-		s = t;
+		s = strreplace(t, '/', '!');
 	}
 	kfree_const(kobj->name);
 	kobj->name = s;

lib/overflow_kunit.c

@@ -649,7 +649,7 @@ struct __test_flex_array {
 static void overflow_size_helpers_test(struct kunit *test)
 {
 	/* Make sure struct_size() can be used in a constant expression. */
-	u8 ce_array[struct_size((struct __test_flex_array *)0, data, 55)];
+	u8 ce_array[struct_size_t(struct __test_flex_array, data, 55)];
 	struct __test_flex_array *obj;
 	int count = 0;
 	int var;

lib/strcat_kunit.c

@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Kernel module for testing 'strcat' family of functions.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <kunit/test.h>
+#include <linux/string.h>
+
+static volatile int unconst;
+
+static void strcat_test(struct kunit *test)
+{
+	char dest[8];
+
+	/* Destination is terminated. */
+	memset(dest, 0, sizeof(dest));
+	KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+	/* Empty copy does nothing. */
+	KUNIT_EXPECT_TRUE(test, strcat(dest, "") == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+	/* 4 characters copied in, stops at %NUL. */
+	KUNIT_EXPECT_TRUE(test, strcat(dest, "four\000123") == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "four");
+	KUNIT_EXPECT_EQ(test, dest[5], '\0');
+	/* 2 more characters copied in okay. */
+	KUNIT_EXPECT_TRUE(test, strcat(dest, "AB") == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+}
+
+static void strncat_test(struct kunit *test)
+{
+	char dest[8];
+
+	/* Destination is terminated. */
+	memset(dest, 0, sizeof(dest));
+	KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+	/* Empty copy of size 0 does nothing. */
+	KUNIT_EXPECT_TRUE(test, strncat(dest, "", 0 + unconst) == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+	/* Empty copy of size 1 does nothing too. */
+	KUNIT_EXPECT_TRUE(test, strncat(dest, "", 1 + unconst) == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+	/* Copy of max 0 characters should do nothing. */
+	KUNIT_EXPECT_TRUE(test, strncat(dest, "asdf", 0 + unconst) == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+
+	/* 4 characters copied in, even if max is 8. */
+	KUNIT_EXPECT_TRUE(test, strncat(dest, "four\000123", 8 + unconst) == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "four");
+	KUNIT_EXPECT_EQ(test, dest[5], '\0');
+	KUNIT_EXPECT_EQ(test, dest[6], '\0');
+	/* 2 characters copied in okay, 2 ignored. */
+	KUNIT_EXPECT_TRUE(test, strncat(dest, "ABCD", 2 + unconst) == dest);
+	KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+}
+
+static void strlcat_test(struct kunit *test)
+{
+	char dest[8] = "";
+	int len = sizeof(dest) + unconst;
+
+	/* Destination is terminated. */
+	KUNIT_EXPECT_EQ(test, strlen(dest), 0);
+	/* Empty copy is size 0. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "", len), 0);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+	/* Size 1 should keep buffer terminated, report size of source only. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "four", 1 + unconst), 4);
+	KUNIT_EXPECT_STREQ(test, dest, "");
+
+	/* 4 characters copied in. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "four", len), 4);
+	KUNIT_EXPECT_STREQ(test, dest, "four");
+	/* 2 characters copied in okay, gets to 6 total. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "AB", len), 6);
+	KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+	/* 2 characters ignored if max size (7) reached. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "CD", 7 + unconst), 8);
+	KUNIT_EXPECT_STREQ(test, dest, "fourAB");
+	/* 1 of 2 characters skipped, now at true max size. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "EFG", len), 9);
+	KUNIT_EXPECT_STREQ(test, dest, "fourABE");
+	/* Everything else ignored, now at full size. */
+	KUNIT_EXPECT_EQ(test, strlcat(dest, "1234", len), 11);
+	KUNIT_EXPECT_STREQ(test, dest, "fourABE");
+}
+
+static struct kunit_case strcat_test_cases[] = {
+	KUNIT_CASE(strcat_test),
+	KUNIT_CASE(strncat_test),
+	KUNIT_CASE(strlcat_test),
+	{}
+};
+
+static struct kunit_suite strcat_test_suite = {
+	.name = "strcat",
+	.test_cases = strcat_test_cases,
+};
+
+kunit_test_suite(strcat_test_suite);
+
+MODULE_LICENSE("GPL");

lib/string.c

@@ -110,7 +110,7 @@ size_t strlcpy(char *dest, const char *src, size_t size)
 
 	if (size) {
 		size_t len = (ret >= size) ? size - 1 : ret;
-		memcpy(dest, src, len);
+		__builtin_memcpy(dest, src, len);
 		dest[len] = '\0';
 	}
 	return ret;
@@ -260,7 +260,7 @@ size_t strlcat(char *dest, const char *src, size_t count)
 	count -= dsize;
 	if (len >= count)
 		len = count-1;
-	memcpy(dest, src, len);
+	__builtin_memcpy(dest, src, len);
 	dest[len] = 0;
 	return res;
 }

lib/string_helpers.c

@@ -979,18 +979,22 @@ EXPORT_SYMBOL(__sysfs_match_string);
 
 /**
  * strreplace - Replace all occurrences of character in string.
- * @s: The string to operate on.
+ * @str: The string to operate on.
  * @old: The character being replaced.
  * @new: The character @old is replaced with.
  *
- * Returns pointer to the nul byte at the end of @s.
+ * Replaces the each @old character with a @new one in the given string @str.
+ *
+ * Return: pointer to the string @str itself.
  */
-char *strreplace(char *s, char old, char new)
+char *strreplace(char *str, char old, char new)
 {
+	char *s = str;
+
 	for (; *s; ++s)
 		if (*s == old)
 			*s = new;
-	return s;
+	return str;
 }
 EXPORT_SYMBOL(strreplace);
 

lib/ubsan.c

@@ -423,9 +423,6 @@ out:
 }
 EXPORT_SYMBOL(__ubsan_handle_load_invalid_value);
 
-void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
-					 unsigned long align,
-					 unsigned long offset);
 void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
 					 unsigned long align,
 					 unsigned long offset)

lib/ubsan.h

@@ -124,4 +124,15 @@ typedef s64 s_max;
 typedef u64 u_max;
 #endif
 
+void __ubsan_handle_divrem_overflow(void *_data, void *lhs, void *rhs);
+void __ubsan_handle_type_mismatch(struct type_mismatch_data *data, void *ptr);
+void __ubsan_handle_type_mismatch_v1(void *_data, void *ptr);
+void __ubsan_handle_out_of_bounds(void *_data, void *index);
+void __ubsan_handle_shift_out_of_bounds(void *_data, void *lhs, void *rhs);
+void __ubsan_handle_builtin_unreachable(void *_data);
+void __ubsan_handle_load_invalid_value(void *_data, void *val);
+void __ubsan_handle_alignment_assumption(void *_data, unsigned long ptr,
+					 unsigned long align,
+					 unsigned long offset);
+
 #endif