Replacing compile-time safe calls of strcpy()-related functions with
strscpy() was always calling the full strscpy() logic when a builtin
would be better. For example:
char buf[16];
strcpy(buf, "yes");
would reduce to __builtin_memcpy(buf, "yes", 4), but not if it was:
strscpy(buf, yes, sizeof(buf));
Fix this by checking if all sizes are known at compile-time.
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
include/linux/fortify-string.h | 10 ++++++++++
lib/strscpy_kunit.c | 13 +++++++++++++
2 files changed, 23 insertions(+)
diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
index aa1a50009632..c473adb55cf5 100644
--- a/include/linux/fortify-string.h
+++ b/include/linux/fortify-string.h
@@ -321,6 +321,16 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
if (__compiletime_lessthan(p_size, size))
__write_overflow();
+ /* Short-circuit for compile-time known-safe lengths. */
+ if (__compiletime_lessthan(p_size, SIZE_MAX)) {
+ len = __compiletime_strlen(q);
+
+ if (len < SIZE_MAX && __compiletime_lessthan(len, size)) {
+ __underlying_memcpy(p, q, len + 1);
+ return len;
+ }
+ }
+
/*
* This call protects from read overflow, because len will default to q
* length if it smaller than size.
diff --git a/lib/strscpy_kunit.c b/lib/strscpy_kunit.c
index 98523f828d3a..a6b6344354ed 100644
--- a/lib/strscpy_kunit.c
+++ b/lib/strscpy_kunit.c
@@ -81,6 +81,8 @@ static void tc(struct kunit *test, char *src, int count, int expected,
static void strscpy_test(struct kunit *test)
{
+ char dest[8];
+
/*
* tc() uses a destination buffer of size 6 and needs at
* least 2 characters spare (one for null and one to check for
@@ -111,6 +113,17 @@ static void strscpy_test(struct kunit *test)
tc(test, "ab", 4, 2, 2, 1, 1);
tc(test, "a", 4, 1, 1, 1, 2);
tc(test, "", 4, 0, 0, 1, 3);
+
+ /* Compile-time-known source strings. */
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "", ARRAY_SIZE(dest)), 0);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "", 3), 0);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "", 1), 0);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "", 0), -E2BIG);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", ARRAY_SIZE(dest)), 5);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 3), -E2BIG);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 1), -E2BIG);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 0), -E2BIG);
+ KUNIT_EXPECT_EQ(test, strscpy(dest, "This is too long", ARRAY_SIZE(dest)), -E2BIG);
}
static struct kunit_case strscpy_test_cases[] = {
--
2.34.1
On Tue, Oct 18, 2022 at 01:32:51AM -0700, Kees Cook wrote:
> Replacing compile-time safe calls of strcpy()-related functions with
> strscpy() was always calling the full strscpy() logic when a builtin
> would be better. For example:
>
> char buf[16];
> strcpy(buf, "yes");
>
> would reduce to __builtin_memcpy(buf, "yes", 4), but not if it was:
>
> strscpy(buf, yes, sizeof(buf));
>
> Fix this by checking if all sizes are known at compile-time.
>
> Cc: [email protected]
> Signed-off-by: Kees Cook <[email protected]>
I ran the following commands and the tests all passed:
$ tools/testing/kunit/kunit.py run --arch arm64 --cross_compile aarch64-linux-gnu- strscpy
$ tools/testing/kunit/kunit.py run --arch arm64 --make_options LLVM=1 strscpy
$ tools/testing/kunit/kunit.py run --arch x86_64 strscpy
$ tools/testing/kunit/kunit.py run --arch x86_64 --make_options LLVM=1 strscpy
$ tools/testing/kunit/kunit.py run --arch arm64 --cross_compile aarch64-linux-gnu- --kconfig_add CONFIG_FORTIFY_SOURCE=y strscpy
$ tools/testing/kunit/kunit.py run --arch arm64 --kconfig_add CONFIG_FORTIFY_SOURCE=y --make_options LLVM=1 strscpy
$ tools/testing/kunit/kunit.py run --arch x86_64 --kconfig_add CONFIG_FORTIFY_SOURCE=y strscpy
$ tools/testing/kunit/kunit.py run --arch x86_64 --kconfig_add CONFIG_FORTIFY_SOURCE=y --make_options LLVM=1 strscpy
Tested-by: Nathan Chancellor <[email protected]>
> ---
> include/linux/fortify-string.h | 10 ++++++++++
> lib/strscpy_kunit.c | 13 +++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index aa1a50009632..c473adb55cf5 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -321,6 +321,16 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
> if (__compiletime_lessthan(p_size, size))
> __write_overflow();
>
> + /* Short-circuit for compile-time known-safe lengths. */
> + if (__compiletime_lessthan(p_size, SIZE_MAX)) {
> + len = __compiletime_strlen(q);
> +
> + if (len < SIZE_MAX && __compiletime_lessthan(len, size)) {
> + __underlying_memcpy(p, q, len + 1);
> + return len;
> + }
> + }
> +
> /*
> * This call protects from read overflow, because len will default to q
> * length if it smaller than size.
> diff --git a/lib/strscpy_kunit.c b/lib/strscpy_kunit.c
> index 98523f828d3a..a6b6344354ed 100644
> --- a/lib/strscpy_kunit.c
> +++ b/lib/strscpy_kunit.c
> @@ -81,6 +81,8 @@ static void tc(struct kunit *test, char *src, int count, int expected,
>
> static void strscpy_test(struct kunit *test)
> {
> + char dest[8];
> +
> /*
> * tc() uses a destination buffer of size 6 and needs at
> * least 2 characters spare (one for null and one to check for
> @@ -111,6 +113,17 @@ static void strscpy_test(struct kunit *test)
> tc(test, "ab", 4, 2, 2, 1, 1);
> tc(test, "a", 4, 1, 1, 1, 2);
> tc(test, "", 4, 0, 0, 1, 3);
> +
> + /* Compile-time-known source strings. */
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "", ARRAY_SIZE(dest)), 0);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "", 3), 0);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "", 1), 0);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "", 0), -E2BIG);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", ARRAY_SIZE(dest)), 5);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 3), -E2BIG);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 1), -E2BIG);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "Fixed", 0), -E2BIG);
> + KUNIT_EXPECT_EQ(test, strscpy(dest, "This is too long", ARRAY_SIZE(dest)), -E2BIG);
> }
>
> static struct kunit_case strscpy_test_cases[] = {
> --
> 2.34.1
>
>