From 2d47c6956ab3c8b580a59d7704aab3e2a4882b6c Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 4 Apr 2023 19:23:59 -0700 Subject: ubsan: Tighten UBSAN_BOUNDS on GCC The use of -fsanitize=bounds on GCC will ignore some trailing arrays, leaving a gap in coverage. Switch to using -fsanitize=bounds-strict to match Clang's stricter behavior. Cc: Marco Elver Cc: Masahiro Yamada Cc: Nathan Chancellor Cc: Nick Desaulniers Cc: Nicolas Schier Cc: Tom Rix Cc: Josh Poimboeuf Cc: Miroslav Benes Cc: linux-kbuild@vger.kernel.org Cc: llvm@lists.linux.dev Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230405022356.gonna.338-kees@kernel.org --- lib/Kconfig.ubsan | 56 ++++++++++++++++++++++++++++++------------------------- 1 file changed, 31 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index fd15230a703b..65d8bbcba438 100644 --- a/lib/Kconfig.ubsan +++ b/lib/Kconfig.ubsan @@ -27,16 +27,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 +57,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) - 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. + def_bool UBSAN_ARRAY_BOUNDS && UBSAN_TRAP + help + 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" -- cgit v1.2.3 From a9dc8d0442294b426b1ebd4ec6097c82ebe282e0 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 26 Mar 2023 13:53:27 -0700 Subject: fortify: Allow KUnit test to build without FORTIFY In order for CI systems to notice all the skipped tests related to CONFIG_FORTIFY_SOURCE, allow the FORTIFY_SOURCE KUnit tests to build with or without CONFIG_FORTIFY_SOURCE. Signed-off-by: Kees Cook --- lib/Kconfig.debug | 2 +- lib/fortify_kunit.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index ce51d4dc6803..1a630a03dfcc 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2645,7 +2645,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 diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index c8c33cbaae9e..524132f33cf0 100644 --- a/lib/fortify_kunit.c +++ b/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, }; -- cgit v1.2.3 From 3bf301e1ab85e18ed0e337ce124dc71d6d7b5fd7 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 4 Apr 2023 15:43:35 -0700 Subject: string: Add Kunit tests for strcat() family Add tests to make sure the strcat() family of functions behave correctly. Signed-off-by: Kees Cook --- lib/Kconfig.debug | 5 +++ lib/Makefile | 1 + lib/strcat_kunit.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+) create mode 100644 lib/strcat_kunit.c (limited to 'lib') diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 1a630a03dfcc..c2a7608ff585 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -2662,6 +2662,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 diff --git a/lib/Makefile b/lib/Makefile index 876fcdeae34e..38c89b52d3ef 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -392,6 +392,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 diff --git a/lib/strcat_kunit.c b/lib/strcat_kunit.c new file mode 100644 index 000000000000..e21be95514af --- /dev/null +++ b/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 +#include + +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"); -- cgit v1.2.3 From 08e4044243a668ea2801cebffcb7c07df568ed3f Mon Sep 17 00:00:00 2001 From: Nick Desaulniers Date: Fri, 7 Apr 2023 14:54:06 -0700 Subject: ubsan: remove cc-option test for UBSAN_TRAP -fsanitize-undefined-trap-on-error has been supported since GCC 5.1 and Clang 3.2. The minimum supported version of these according to Documentation/process/changes.rst is 5.1 and 11.0.0 respectively. Drop this cc-option check. Signed-off-by: Nick Desaulniers Reviewed-by: Nathan Chancellor Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230407215406.768464-1-ndesaulniers@google.com --- lib/Kconfig.ubsan | 1 - 1 file changed, 1 deletion(-) (limited to 'lib') diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index 65d8bbcba438..efae7e011956 100644 --- a/lib/Kconfig.ubsan +++ b/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 -- cgit v1.2.3 From d67790ddf0219aa0ad3e13b53ae0a7619b3425a2 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 22 May 2023 14:18:13 -0700 Subject: overflow: Add struct_size_t() helper While struct_size() is normally used in situations where the structure type already has a pointer instance, there are places where no variable is available. In the past, this has been worked around by using a typed NULL first argument, but this is a bit ugly. Add a helper to do this, and replace the handful of instances of the code pattern with it. Instances were found with this Coccinelle script: @struct_size_t@ identifier STRUCT, MEMBER; expression COUNT; @@ - struct_size((struct STRUCT *)\(0\|NULL\), + struct_size_t(struct STRUCT, MEMBER, COUNT) Suggested-by: Christoph Hellwig Cc: Jesse Brandeburg Cc: Tony Nguyen Cc: "David S. Miller" Cc: Eric Dumazet Cc: Paolo Abeni Cc: James Smart Cc: Keith Busch Cc: Jens Axboe Cc: Sagi Grimberg Cc: HighPoint Linux Team Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: Kashyap Desai Cc: Sumit Saxena Cc: Shivasharan S Cc: Don Brace Cc: "Darrick J. Wong" Cc: Dave Chinner Cc: Guo Xuenan Cc: Gwan-gyeong Mun Cc: Nick Desaulniers Cc: Daniel Latypov Cc: kernel test robot Cc: intel-wired-lan@lists.osuosl.org Cc: netdev@vger.kernel.org Cc: linux-nvme@lists.infradead.org Cc: linux-scsi@vger.kernel.org Cc: megaraidlinux.pdl@broadcom.com Cc: storagedev@microchip.com Cc: linux-xfs@vger.kernel.org Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook Acked-by: Martin K. Petersen Reviewed-by: Darrick J. Wong Reviewed-by: Gustavo A. R. Silva Reviewed-by: Christoph Hellwig Acked-by: Jakub Kicinski Reviewed-by: Alexander Lobakin Link: https://lore.kernel.org/r/20230522211810.never.421-kees@kernel.org --- lib/overflow_kunit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/overflow_kunit.c b/lib/overflow_kunit.c index dcd3ba102db6..34db0b3aa502 100644 --- a/lib/overflow_kunit.c +++ b/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; -- cgit v1.2.3 From 26f15e5de15f7917358af0fda4291b74a3285989 Mon Sep 17 00:00:00 2001 From: Arnd Bergmann Date: Wed, 17 May 2023 14:50:34 +0200 Subject: ubsan: add prototypes for internal functions Most of the functions in ubsan that are only called from generated code don't have a prototype, which W=1 builds warn about: lib/ubsan.c:226:6: error: no previous prototype for '__ubsan_handle_divrem_overflow' [-Werror=missing-prototypes] lib/ubsan.c:307:6: error: no previous prototype for '__ubsan_handle_type_mismatch' [-Werror=missing-prototypes] lib/ubsan.c:321:6: error: no previous prototype for '__ubsan_handle_type_mismatch_v1' [-Werror=missing-prototypes] lib/ubsan.c:335:6: error: no previous prototype for '__ubsan_handle_out_of_bounds' [-Werror=missing-prototypes] lib/ubsan.c:352:6: error: no previous prototype for '__ubsan_handle_shift_out_of_bounds' [-Werror=missing-prototypes] lib/ubsan.c:394:6: error: no previous prototype for '__ubsan_handle_builtin_unreachable' [-Werror=missing-prototypes] lib/ubsan.c:404:6: error: no previous prototype for '__ubsan_handle_load_invalid_value' [-Werror=missing-prototypes] Add prototypes for all of these to lib/ubsan.h, and remove the one that was already present in ubsan.c. Signed-off-by: Arnd Bergmann Reviewed-by: Fangrui Song Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230517125102.930491-1-arnd@kernel.org --- lib/ubsan.c | 3 --- lib/ubsan.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) (limited to 'lib') diff --git a/lib/ubsan.c b/lib/ubsan.c index e2cc4a799312..3f90810f9f42 100644 --- a/lib/ubsan.c +++ b/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) diff --git a/lib/ubsan.h b/lib/ubsan.h index cc5cb94895a6..5d99ab81913b 100644 --- a/lib/ubsan.h +++ b/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 -- cgit v1.2.3 From f9cfb1910ece5b5dbedca096fc9b7c9fe4fd3c50 Mon Sep 17 00:00:00 2001 From: Alexander Potapenko Date: Tue, 30 May 2023 10:39:11 +0200 Subject: string: use __builtin_memcpy() in strlcpy/strlcat lib/string.c is built with -ffreestanding, which prevents the compiler from replacing certain functions with calls to their library versions. On the other hand, this also prevents Clang and GCC from instrumenting calls to memcpy() when building with KASAN, KCSAN or KMSAN: - KASAN normally replaces memcpy() with __asan_memcpy() with the additional cc-param,asan-kernel-mem-intrinsic-prefix=1; - KCSAN and KMSAN replace memcpy() with __tsan_memcpy() and __msan_memcpy() by default. To let the tools catch memory accesses from strlcpy/strlcat, replace the calls to memcpy() with __builtin_memcpy(), which KASAN, KCSAN and KMSAN are able to replace even in -ffreestanding mode. This preserves the behavior in normal builds (__builtin_memcpy() ends up being replaced with memcpy()), and does not introduce new instrumentation in unwanted places, as strlcpy/strlcat are already instrumented. Suggested-by: Marco Elver Signed-off-by: Alexander Potapenko Reviewed-by: Marco Elver Link: https://lore.kernel.org/all/20230224085942.1791837-1-elver@google.com/ Acked-by: Kees Cook Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230530083911.1104336-1-glider@google.com --- lib/string.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/string.c b/lib/string.c index 3d55ef890106..be26623953d2 100644 --- a/lib/string.c +++ b/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; } -- cgit v1.2.3 From d01a77afd6bef1b3a2ed15e8ca6887ca7da0cddc Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 5 Jun 2023 20:05:52 +0300 Subject: lib/string_helpers: Change returned value of the strreplace() It's more useful to return the pointer to the string itself with strreplace(), so it may be used like attr->name = strreplace(name, '/', '_'); While at it, amend the kernel documentation. Signed-off-by: Andy Shevchenko Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230605170553.7835-3-andriy.shevchenko@linux.intel.com --- lib/string_helpers.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/string_helpers.c b/lib/string_helpers.c index 230020a2e076..d3b1dd718daf 100644 --- a/lib/string_helpers.c +++ b/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); -- cgit v1.2.3 From b2f10148ec1eae7d63dd6a1a56afdf93a27daa74 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Mon, 5 Jun 2023 20:05:53 +0300 Subject: kobject: Use return value of strreplace() Since strreplace() returns the pointer to the string itself, we may use it directly in the code. Signed-off-by: Andy Shevchenko Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20230605170553.7835-4-andriy.shevchenko@linux.intel.com --- lib/kobject.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/kobject.c b/lib/kobject.c index f79a434e1231..16d530f9c174 100644 --- a/lib/kobject.c +++ b/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; -- cgit v1.2.3