fortify: Short-circuit known-safe calls to strscpy()
authorKees Cook <keescook@chromium.org>
Sun, 2 Oct 2022 16:17:03 +0000 (09:17 -0700)
committerKees Cook <keescook@chromium.org>
Tue, 1 Nov 2022 17:04:52 +0000 (10:04 -0700)
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: linux-hardening@vger.kernel.org
Tested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
include/linux/fortify-string.h
lib/strscpy_kunit.c

index 49782f6..32a66d4 100644 (file)
@@ -314,6 +314,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.
index 98523f8..a6b6344 100644 (file)
@@ -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[] = {