Whamcloud - gitweb
LU-16510 build: fortified memcpy from linux 6.1 15/49815/5
authorShaun Tancheff <shaun.tancheff@hpe.com>
Thu, 9 Feb 2023 03:50:05 +0000 (19:50 -0800)
committerOleg Drokin <green@whamcloud.com>
Tue, 28 Mar 2023 07:13:55 +0000 (07:13 +0000)
The fortified memcpy() from Linux v5.11-11104-ga28a6e860c6c
through v5.18-rc5-1405-g43213daed6d6 incorrectly reports
a false positive out of bounds check.

In function 'memcpy' ...
  '__read_overflow2' declared with attribute error: detected
   read beyond size of object passed as 2nd parameter

Lustre-change: https://review.whamcloud.com/49811
Lustre-commit: 919b93b951d4a9aa0400b9c882a1f68b79d8f118

Test-Parameters: trivial
HPE-bug-id: LUS-11459
Signed-off-by: Shaun Tancheff <shaun.tancheff@hpe.com>
Change-Id: I3a59d8b647833c05ff4b51e327ed8bce894141fe
Reviewed-by: Jian Yu <yujian@whamcloud.com>
Reviewed-by: Yang Sheng <ys@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/49815
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
libcfs/autoconf/lustre-libcfs.m4
libcfs/include/libcfs/libcfs.h
libcfs/include/libcfs/linux/Makefile.am
libcfs/include/libcfs/linux/linux-fortify-string.h [new file with mode: 0644]

index 75b1e5b..66c97eb 100644 (file)
@@ -2006,6 +2006,26 @@ AC_DEFUN([LIBCFS_NLA_STRLCPY], [
 ]) # LIBCFS_NLA_STRLCPY
 
 #
+# LIBCFS_LINUX_FORTIFY_STRING_HEADER
+#
+# Linux v5.11-11104-ga28a6e860c6c
+#  string.h: move fortified functions definitions in a dedicated header.
+#
+AC_DEFUN([LIBCFS_SRC_LINUX_FORTIFY_STRING_HEADER],[
+       LB2_LINUX_TEST_SRC([linux_fortify_string_header], [
+               #include <linux/fortify-string.h>
+       ],[
+       ],[])
+])
+AC_DEFUN([LIBCFS_LINUX_FORTIFY_STRING_HEADER],[
+       AC_MSG_CHECKING([Is linux/fortify-string.h header available])
+       LB2_LINUX_TEST_RESULT([linux_fortify_string_header], [
+               AC_DEFINE(HAVE_LINUX_FORTIFY_STRING_HEADER, 1,
+                       [linux/fortify-string.h header available])
+       ])
+]) # LIBCFS_LINUX_FORTIFY_STRING_HEADER
+
+#
 # LIBCFS_HAVE_CIPHER_HEADER
 #
 # Kernel 5.12 commit 0eb76ba29d16df2951d37c54ca279c4e5630b071
@@ -2289,6 +2309,8 @@ AC_DEFUN([LIBCFS_PROG_LINUX_SRC], [
        LIBCFS_SRC_HAVE_KFREE_SENSITIVE
        LIBCFS_SRC_HAVE_LIST_CMP_FUNC_T
        LIBCFS_SRC_NLA_STRLCPY
+       # 5.12
+       LIBCFS_SRC_LINUX_FORTIFY_STRING_HEADER
        # 5.13
        LIBCFS_SRC_HAVE_TASK_IS_RUNNING
        LIBCFS_SRC_IOV_ITER_HAS_ITER_TYPE
@@ -2426,6 +2448,8 @@ AC_DEFUN([LIBCFS_PROG_LINUX_RESULTS], [
        LIBCFS_HAVE_KFREE_SENSITIVE
        LIBCFS_HAVE_LIST_CMP_FUNC_T
        LIBCFS_NLA_STRLCPY
+       # 5.12
+       LIBCFS_LINUX_FORTIFY_STRING_HEADER
        # 5.13
        LIBCFS_HAVE_TASK_IS_RUNNING
        LIBCFS_IOV_ITER_HAS_ITER_TYPE
index 601ebfe..11bbaaa 100644 (file)
@@ -44,6 +44,7 @@
 #include <libcfs/linux/linux-mem.h>
 #include <libcfs/linux/linux-time.h>
 #include <libcfs/linux/linux-wait.h>
+#include <libcfs/linux/linux-fortify-string.h>
 
 #include <uapi/linux/lnet/libcfs_ioctl.h>
 #include <libcfs/libcfs_debug.h>
index d3fedf6..5003e4b 100644 (file)
@@ -1,3 +1,3 @@
 EXTRA_DIST = linux-misc.h linux-fs.h linux-mem.h linux-time.h linux-cpu.h \
             linux-list.h linux-hash.h linux-uuid.h linux-wait.h linux-net.h \
-            glob.h refcount.h processor.h xarray.h
+            glob.h refcount.h processor.h xarray.h linux-fortify-string.h
diff --git a/libcfs/include/libcfs/linux/linux-fortify-string.h b/libcfs/include/libcfs/linux/linux-fortify-string.h
new file mode 100644 (file)
index 0000000..bf03e0e
--- /dev/null
@@ -0,0 +1,280 @@
+#ifndef _LIBCFS_FORTIFY_STRING_H
+#define _LIBCFS_FORTIFY_STRING_H
+
+#ifdef HAVE_LINUX_FORTIFY_STRING_HEADER
+#include <linux/fortify-string.h>
+
+/*
+ * Linux v5.11-11104-ga28a6e860c6c introduces fortify-string.h
+ * where an unsafe_memcpy is provided in Linux v5.18-rc5-1405-g43213daed6d6
+ *
+ * This following is excerpted from the Linux v6.1 fortified memcpy()
+ * which resolves some corner cases, one of which is triggered in lustre
+ */
+#ifndef unsafe_memcpy
+
+#include <linux/bug.h>
+#include <linux/const.h>
+#include <linux/limits.h>
+
+#ifndef __RENAME
+#define __RENAME(x) __asm__(#x)
+#endif
+
+void fortify_panic(const char *name) __noreturn __cold;
+void __read_overflow(void) __compiletime_error("detected read beyond size of object (1st parameter)");
+void __read_overflow2(void) __compiletime_error("detected read beyond size of object (2nd parameter)");
+void __read_overflow2_field(size_t avail, size_t wanted) __compiletime_warning("detected read beyond size of field (2nd parameter); maybe use struct_group()?");
+void __write_overflow(void) __compiletime_error("detected write beyond size of object (1st parameter)");
+void __write_overflow_field(size_t avail, size_t wanted) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");
+
+#define __compiletime_strlen(p)                                        \
+({                                                             \
+       char *__p = (char *)(p);                                \
+       size_t __ret = SIZE_MAX;                                \
+       size_t __p_size = __member_size(p);                     \
+       if (__p_size != SIZE_MAX &&                             \
+           __builtin_constant_p(*__p)) {                       \
+               size_t __p_len = __p_size - 1;                  \
+               if (__builtin_constant_p(__p[__p_len]) &&       \
+                   __p[__p_len] == '\0')                       \
+                       __ret = __builtin_strlen(__p);          \
+       }                                                       \
+       __ret;                                                  \
+})
+
+#if defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)
+extern void *__underlying_memchr(const void *p, int c, __kernel_size_t size) __RENAME(memchr);
+extern int __underlying_memcmp(const void *p, const void *q, __kernel_size_t size) __RENAME(memcmp);
+extern void *__underlying_memcpy(void *p, const void *q, __kernel_size_t size) __RENAME(memcpy);
+extern void *__underlying_memmove(void *p, const void *q, __kernel_size_t size) __RENAME(memmove);
+extern void *__underlying_memset(void *p, int c, __kernel_size_t size) __RENAME(memset);
+extern char *__underlying_strcat(char *p, const char *q) __RENAME(strcat);
+extern char *__underlying_strcpy(char *p, const char *q) __RENAME(strcpy);
+extern __kernel_size_t __underlying_strlen(const char *p) __RENAME(strlen);
+extern char *__underlying_strncat(char *p, const char *q, __kernel_size_t count) __RENAME(strncat);
+extern char *__underlying_strncpy(char *p, const char *q, __kernel_size_t size) __RENAME(strncpy);
+#else
+
+#if defined(__SANITIZE_MEMORY__)
+/*
+ * For KMSAN builds all memcpy/memset/memmove calls should be replaced by the
+ * corresponding __msan_XXX functions.
+ */
+#include <linux/kmsan_string.h>
+#define __underlying_memcpy    __msan_memcpy
+#define __underlying_memmove   __msan_memmove
+#define __underlying_memset    __msan_memset
+#else
+#define __underlying_memcpy    __builtin_memcpy
+#define __underlying_memmove   __builtin_memmove
+#define __underlying_memset    __builtin_memset
+#endif
+
+#define __underlying_memchr    __builtin_memchr
+#define __underlying_memcmp    __builtin_memcmp
+#define __underlying_strcat    __builtin_strcat
+#define __underlying_strcpy    __builtin_strcpy
+#define __underlying_strlen    __builtin_strlen
+#define __underlying_strncat   __builtin_strncat
+#define __underlying_strncpy   __builtin_strncpy
+#endif
+
+/*
+ * Clang's use of __builtin_*object_size() within inlines needs hinting via
+ * __pass_*object_size(). The preference is to only ever use type 1 (member
+ * size, rather than struct size), but there remain some stragglers using
+ * type 0 that will be converted in the future.
+ */
+#define POS                    __pass_object_size(1)
+#define POS0                   __pass_object_size(0)
+#define __struct_size(p)       __builtin_object_size(p, 0)
+#define __member_size(p)       __builtin_object_size(p, 1)
+
+#define __compiletime_lessthan(bounds, length) (       \
+       __builtin_constant_p((bounds) < (length)) &&    \
+       (bounds) < (length)                             \
+)
+
+
+/*
+ * To make sure the compiler can enforce protection against buffer overflows,
+ * memcpy(), memmove(), and memset() must not be used beyond individual
+ * struct members. If you need to copy across multiple members, please use
+ * struct_group() to create a named mirror of an anonymous struct union.
+ * (e.g. see struct sk_buff.) Read overflow checking is currently only
+ * done when a write overflow is also present, or when building with W=1.
+ *
+ * Mitigation coverage matrix
+ *                                     Bounds checking at:
+ *                                     +-------+-------+-------+-------+
+ *                                     | Compile time  |   Run time    |
+ * memcpy() argument sizes:            | write | read  | write | read  |
+ *        dest     source   length      +-------+-------+-------+-------+
+ * memcpy(known,   known,   constant)  |   y   |   y   |  n/a  |  n/a  |
+ * memcpy(known,   unknown, constant)  |   y   |   n   |  n/a  |   V   |
+ * memcpy(known,   known,   dynamic)   |   n   |   n   |   B   |   B   |
+ * memcpy(known,   unknown, dynamic)   |   n   |   n   |   B   |   V   |
+ * memcpy(unknown, known,   constant)  |   n   |   y   |   V   |  n/a  |
+ * memcpy(unknown, unknown, constant)  |   n   |   n   |   V   |   V   |
+ * memcpy(unknown, known,   dynamic)   |   n   |   n   |   V   |   B   |
+ * memcpy(unknown, unknown, dynamic)   |   n   |   n   |   V   |   V   |
+ *                                     +-------+-------+-------+-------+
+ *
+ * y = perform deterministic compile-time bounds checking
+ * n = cannot perform deterministic compile-time bounds checking
+ * n/a = no run-time bounds checking needed since compile-time deterministic
+ * B = can perform run-time bounds checking (currently unimplemented)
+ * V = vulnerable to run-time overflow (will need refactoring to solve)
+ *
+ */
+extern __always_inline __gnu_inline
+bool fortify_memcpy_chk(__kernel_size_t size,
+                                        const size_t p_size,
+                                        const size_t q_size,
+                                        const size_t p_size_field,
+                                        const size_t q_size_field,
+                                        const char *func)
+{
+       if (__builtin_constant_p(size)) {
+               /*
+                * Length argument is a constant expression, so we
+                * can perform compile-time bounds checking where
+                * buffer sizes are also known at compile time.
+                */
+
+               /* Error when size is larger than enclosing struct. */
+               if (__compiletime_lessthan(p_size_field, p_size) &&
+                   __compiletime_lessthan(p_size, size))
+                       __write_overflow();
+               if (__compiletime_lessthan(q_size_field, q_size) &&
+                   __compiletime_lessthan(q_size, size))
+                       __read_overflow2();
+
+               /* Warn when write size argument larger than dest field. */
+               if (__compiletime_lessthan(p_size_field, size))
+                       __write_overflow_field(p_size_field, size);
+               /*
+                * Warn for source field over-read when building with W=1
+                * or when an over-write happened, so both can be fixed at
+                * the same time.
+                */
+               if ((IS_ENABLED(KBUILD_EXTRA_WARN1) ||
+                    __compiletime_lessthan(p_size_field, size)) &&
+                   __compiletime_lessthan(q_size_field, size))
+                       __read_overflow2_field(q_size_field, size);
+       }
+       /*
+        * At this point, length argument may not be a constant expression,
+        * so run-time bounds checking can be done where buffer sizes are
+        * known. (This is not an "else" because the above checks may only
+        * be compile-time warnings, and we want to still warn for run-time
+        * overflows.)
+        */
+
+       /*
+        * Always stop accesses beyond the struct that contains the
+        * field, when the buffer's remaining size is known.
+        * (The SIZE_MAX test is to optimize away checks where the buffer
+        * lengths are unknown.)
+        */
+       if ((p_size != SIZE_MAX && p_size < size) ||
+           (q_size != SIZE_MAX && q_size < size))
+               fortify_panic(func);
+
+       /*
+        * Warn when writing beyond destination field size.
+        *
+        * We must ignore p_size_field == 0 for existing 0-element
+        * fake flexible arrays, until they are all converted to
+        * proper flexible arrays.
+        *
+        * The implementation of __builtin_*object_size() behaves
+        * like sizeof() when not directly referencing a flexible
+        * array member, which means there will be many bounds checks
+        * that will appear at run-time, without a way for them to be
+        * detected at compile-time (as can be done when the destination
+        * is specifically the flexible array member).
+        * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101832
+        */
+       if (p_size_field != 0 && p_size_field != SIZE_MAX &&
+           p_size != p_size_field && p_size_field < size)
+               return true;
+
+       return false;
+}
+
+#define __fortify_memcpy_chk(p, q, size, p_size, q_size,               \
+                            p_size_field, q_size_field, op) ({         \
+       const size_t __fortify_size = (size_t)(size);                   \
+       const size_t __p_size = (p_size);                               \
+       const size_t __q_size = (q_size);                               \
+       const size_t __p_size_field = (p_size_field);                   \
+       const size_t __q_size_field = (q_size_field);                   \
+       WARN_ONCE(fortify_memcpy_chk(__fortify_size, __p_size,          \
+                                    __q_size, __p_size_field,          \
+                                    __q_size_field, #op),              \
+                 #op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \
+                 __fortify_size,                                       \
+                 "field \"" #p "\" at " __FILE__ ":" __stringify(__LINE__), \
+                 __p_size_field);                                      \
+       __underlying_##op(p, q, __fortify_size);                        \
+})
+
+/*
+ * Notes about compile-time buffer size detection:
+ *
+ * With these types...
+ *
+ *     struct middle {
+ *             u16 a;
+ *             u8 middle_buf[16];
+ *             int b;
+ *     };
+ *     struct end {
+ *             u16 a;
+ *             u8 end_buf[16];
+ *     };
+ *     struct flex {
+ *             int a;
+ *             u8 flex_buf[];
+ *     };
+ *
+ *     void func(TYPE *ptr) { ... }
+ *
+ * Cases where destination size cannot be currently detected:
+ * - the size of ptr's object (seemingly by design, gcc & clang fail):
+ *     __builtin_object_size(ptr, 1) == SIZE_MAX
+ * - the size of flexible arrays in ptr's obj (by design, dynamic size):
+ *     __builtin_object_size(ptr->flex_buf, 1) == SIZE_MAX
+ * - the size of ANY array at the end of ptr's obj (gcc and clang bug):
+ *     __builtin_object_size(ptr->end_buf, 1) == SIZE_MAX
+ *     https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836
+ *
+ * Cases where destination size is currently detected:
+ * - the size of non-array members within ptr's object:
+ *     __builtin_object_size(ptr->a, 1) == 2
+ * - the size of non-flexible-array in the middle of ptr's obj:
+ *     __builtin_object_size(ptr->middle_buf, 1) == 16
+ *
+ */
+
+/*
+ * __struct_size() vs __member_size() must be captured here to avoid
+ * evaluating argument side-effects further into the macro layers.
+ */
+#define memcpy(p, q, s)  __fortify_memcpy_chk(p, q, s,                 \
+               __struct_size(p), __struct_size(q),                     \
+               __member_size(p), __member_size(q),                     \
+               memcpy)
+
+#endif /* HAVE_LINUX_FORTIFY_STRING_HEADER */
+#endif /* unsafe_memcpy */
+
+/* a catch all to ensure an unsafe_memcpy() exists */
+#ifndef unsafe_memcpy
+#define unsafe_memcpy(dst, src, bytes, justification)          \
+       memcpy(dst, src, bytes)
+#endif
+
+#endif /* _LIBCFS_FORTIFY_STRING_H */