All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: linux-hardening@vger.kernel.org,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	Keith Packard <keithpac@amazon.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
	netdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-staging@lists.linux.dev, linux-block@vger.kernel.org,
	linux-kbuild@vger.kernel.org, clang-built-linux@googlegroups.com
Subject: Re: [PATCH 34/64] fortify: Detect struct member overflows in memcpy() at compile-time
Date: Tue, 27 Jul 2021 15:43:27 -0700	[thread overview]
Message-ID: <CAKwvOdknit8DtWaFvLupmNEebjbwVa6R3xiGc2D4AqB_6+i52g@mail.gmail.com> (raw)
In-Reply-To: <20210727205855.411487-35-keescook@chromium.org>

On Tue, Jul 27, 2021 at 2:17 PM Kees Cook <keescook@chromium.org> wrote:
>
> To accelerate the review of potential run-time false positives, it's
> also worth noting that it is possible to partially automate checking
> by examining memcpy() buffer argument fields to see if they have
> a neighboring. It is reasonable to expect that the vast majority of

a neighboring...field?

> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 7e67d02764db..5e79e626172b 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,13 +2,17 @@
>  #ifndef _LINUX_FORTIFY_STRING_H_
>  #define _LINUX_FORTIFY_STRING_H_
>
> +#include <linux/bug.h>

What are you using from linux/bug.h here?

> +
>  #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
>  #define __RENAME(x) __asm__(#x)
>
>  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(void) __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(void) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");
>
>  #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);
> @@ -182,22 +186,105 @@ __FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
>         return __underlying_memset(p, c, size);
>  }
>
> -__FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
> +/*
> + * 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.)
> + *
> + * Mitigation coverage
> + *                                     Bounds checking at:
> + *                                     +-------+-------+-------+-------+
> + *                                     | Compile time  | Run time      |
> + * memcpy() argument sizes:            | write | read  | write | read  |
> + *                                     +-------+-------+-------+-------+
> + * memcpy(known,   known,   constant)  |   y   |   y   |  n/a  |  n/a  |
> + * memcpy(unknown, known,   constant)  |   n   |   y   |   V   |  n/a  |
> + * memcpy(known,   unknown, constant)  |   y   |   n   |  n/a  |   V   |
> + * memcpy(unknown, unknown, constant)  |   n   |   n   |   V   |   V   |
> + * memcpy(known,   known,   dynamic)   |   n   |   n   |   b   |   B   |
> + * memcpy(unknown, known,   dynamic)   |   n   |   n   |   V   |   B   |
> + * memcpy(known,   unknown, dynamic)   |   n   |   n   |   b   |   V   |
> + * memcpy(unknown, unknown, dynamic)   |   n   |   n   |   V   |   V   |
> + *                                     +-------+-------+-------+-------+
> + *
> + * y = deterministic compile-time bounds checking
> + * n = cannot do deterministic compile-time bounds checking
> + * n/a = no run-time bounds checking needed since compile-time deterministic
> + * b = perform run-time bounds checking
> + * B = can perform run-time bounds checking, but current unenforced
> + * V = vulnerable to run-time overflow
> + *
> + */
> +__FORTIFY_INLINE void 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)
>  {
> -       size_t p_size = __builtin_object_size(p, 0);
> -       size_t q_size = __builtin_object_size(q, 0);
> -
>         if (__builtin_constant_p(size)) {
> -               if (p_size < size)
> +               /*
> +                * Length argument is a constant expression, so we
> +                * can perform compile-time bounds checking where
> +                * buffer sizes are known.
> +                */
> +
> +               /* Error when size is larger than enclosing struct. */
> +               if (p_size > p_size_field && p_size < size)
>                         __write_overflow();
> -               if (q_size < size)
> +               if (q_size > q_size_field && q_size < size)
>                         __read_overflow2();
> +
> +               /* Warn when write size argument larger than dest field. */
> +               if (p_size_field < size)
> +                       __write_overflow_field();
> +               /*
> +                * 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) || p_size_field < size) &&
> +                   q_size_field < size)
> +                       __read_overflow2_field();
>         }
> -       if (p_size < size || q_size < size)
> -               fortify_panic(__func__);
> -       return __underlying_memcpy(p, q, 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 -1 test is to optimize away checks where the buffer
> +        * lengths are unknown.)
> +        */
> +       if ((p_size != (size_t)(-1) && p_size < size) ||
> +           (q_size != (size_t)(-1) && q_size < size))
> +               fortify_panic(func);
>  }
>
> +#define __fortify_memcpy_chk(p, q, size, p_size, q_size,               \
> +                            p_size_field, q_size_field, op) ({         \
> +       size_t __fortify_size = (size_t)(size);                         \
> +       fortify_memcpy_chk(__fortify_size, p_size, q_size,              \
> +                          p_size_field, q_size_field, #op);            \
> +       __underlying_##op(p, q, __fortify_size);                        \
> +})

Are there other macro expansion sites for `__fortify_memcpy_chk`,
perhaps later in this series? I don't understand why `memcpy` is
passed as `func` to `fortify_panic()` rather than continuing to use
`__func__`?

> +
> +/*
> + * __builtin_object_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,                 \
> +               __builtin_object_size(p, 0), __builtin_object_size(q, 0), \
> +               __builtin_object_size(p, 1), __builtin_object_size(q, 1), \
> +               memcpy)
> +
>  __FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
> @@ -277,27 +364,27 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
>         return __real_kmemdup(p, size, gfp);
>  }
>
> -/* defined after fortified strlen and memcpy to reuse them */
> +/* Defined after fortified strlen to reuse it. */
>  __FORTIFY_INLINE char *strcpy(char *p, const char *q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
>         size_t size;
>
> +       /* If neither buffer size is known, immediately give up. */
>         if (p_size == (size_t)-1 && q_size == (size_t)-1)
>                 return __underlying_strcpy(p, q);
>         size = strlen(q) + 1;
>         /* test here to use the more stringent object size */
>         if (p_size < size)
>                 fortify_panic(__func__);
> -       memcpy(p, q, size);
> +       __underlying_memcpy(p, q, size);
>         return p;
>  }
>
>  /* Don't use these outside the FORITFY_SOURCE implementation */
>  #undef __underlying_memchr
>  #undef __underlying_memcmp
> -#undef __underlying_memcpy
>  #undef __underlying_memmove
>  #undef __underlying_memset
>  #undef __underlying_strcat
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 9473f81b9db2..cbe889e404e2 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -261,8 +261,9 @@ static inline const char *kbasename(const char *path)
>   * @count: The number of bytes to copy
>   * @pad: Character to use for padding if space is left in destination.
>   */
> -static inline void memcpy_and_pad(void *dest, size_t dest_len,
> -                                 const void *src, size_t count, int pad)
> +static __always_inline void memcpy_and_pad(void *dest, size_t dest_len,
> +                                          const void *src, size_t count,
> +                                          int pad)

Why __always_inline here?

>  {
>         if (dest_len > count) {
>                 memcpy(dest, src, count);
> diff --git a/lib/Makefile b/lib/Makefile
> index 083a19336e20..74523fd394bd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -370,7 +370,8 @@ TEST_FORTIFY_LOG = test_fortify.log
>  quiet_cmd_test_fortify = TEST    $@
>        cmd_test_fortify = $(CONFIG_SHELL) $(srctree)/scripts/test_fortify.sh \
>                         $< $@ "$(NM)" $(CC) $(c_flags) \
> -                       $(call cc-disable-warning,fortify-source)
> +                       $(call cc-disable-warning,fortify-source) \
> +                       -DKBUILD_EXTRA_WARN1
>
>  targets += $(TEST_FORTIFY_LOGS)
>  clean-files += $(TEST_FORTIFY_LOGS)
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index faa9d8e4e2c5..4d205bf5993c 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -884,6 +884,12 @@ char *strreplace(char *s, char old, char new)
>  EXPORT_SYMBOL(strreplace);
>
>  #ifdef CONFIG_FORTIFY_SOURCE
> +/* These are placeholders for fortify compile-time warnings. */
> +void __read_overflow2_field(void) { }
> +EXPORT_SYMBOL(__read_overflow2_field);
> +void __write_overflow_field(void) { }
> +EXPORT_SYMBOL(__write_overflow_field);
> +

Don't we rely on these being undefined for Clang to produce a linkage
failure (until https://reviews.llvm.org/D106030 has landed)?  By
providing a symbol definition we can link against, I don't think
__compiletime_{warning|error} will warn at all with Clang?

>  void fortify_panic(const char *name)
>  {
>         pr_emerg("detected buffer overflow in %s\n", name);
> diff --git a/lib/test_fortify/read_overflow2_field-memcpy.c b/lib/test_fortify/read_overflow2_field-memcpy.c
> new file mode 100644
> index 000000000000..de9569266223
> --- /dev/null
> +++ b/lib/test_fortify/read_overflow2_field-memcpy.c
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define TEST   \
> +       memcpy(large, instance.buf, sizeof(instance.buf) + 1)
> +
> +#include "test_fortify.h"
> diff --git a/lib/test_fortify/write_overflow_field-memcpy.c b/lib/test_fortify/write_overflow_field-memcpy.c
> new file mode 100644
> index 000000000000..28cc81058dd3
> --- /dev/null
> +++ b/lib/test_fortify/write_overflow_field-memcpy.c
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define TEST   \
> +       memcpy(instance.buf, large, sizeof(instance.buf) + 1)
> +
> +#include "test_fortify.h"
> --

I haven't read the whole series yet, but I assume test_fortify.h was
provided earlier in the series?
-- 
Thanks,
~Nick Desaulniers

WARNING: multiple messages have this Message-ID (diff)
From: Nick Desaulniers <ndesaulniers@google.com>
To: Kees Cook <keescook@chromium.org>
Cc: linux-kbuild@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-staging@lists.linux.dev, linux-wireless@vger.kernel.org,
	linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-block@vger.kernel.org, clang-built-linux@googlegroups.com,
	Keith Packard <keithpac@amazon.com>,
	linux-hardening@vger.kernel.org, netdev@vger.kernel.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 34/64] fortify: Detect struct member overflows in memcpy() at compile-time
Date: Tue, 27 Jul 2021 15:43:27 -0700	[thread overview]
Message-ID: <CAKwvOdknit8DtWaFvLupmNEebjbwVa6R3xiGc2D4AqB_6+i52g@mail.gmail.com> (raw)
In-Reply-To: <20210727205855.411487-35-keescook@chromium.org>

On Tue, Jul 27, 2021 at 2:17 PM Kees Cook <keescook@chromium.org> wrote:
>
> To accelerate the review of potential run-time false positives, it's
> also worth noting that it is possible to partially automate checking
> by examining memcpy() buffer argument fields to see if they have
> a neighboring. It is reasonable to expect that the vast majority of

a neighboring...field?

> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 7e67d02764db..5e79e626172b 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -2,13 +2,17 @@
>  #ifndef _LINUX_FORTIFY_STRING_H_
>  #define _LINUX_FORTIFY_STRING_H_
>
> +#include <linux/bug.h>

What are you using from linux/bug.h here?

> +
>  #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
>  #define __RENAME(x) __asm__(#x)
>
>  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(void) __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(void) __compiletime_warning("detected write beyond size of field (1st parameter); maybe use struct_group()?");
>
>  #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);
> @@ -182,22 +186,105 @@ __FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
>         return __underlying_memset(p, c, size);
>  }
>
> -__FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
> +/*
> + * 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.)
> + *
> + * Mitigation coverage
> + *                                     Bounds checking at:
> + *                                     +-------+-------+-------+-------+
> + *                                     | Compile time  | Run time      |
> + * memcpy() argument sizes:            | write | read  | write | read  |
> + *                                     +-------+-------+-------+-------+
> + * memcpy(known,   known,   constant)  |   y   |   y   |  n/a  |  n/a  |
> + * memcpy(unknown, known,   constant)  |   n   |   y   |   V   |  n/a  |
> + * memcpy(known,   unknown, constant)  |   y   |   n   |  n/a  |   V   |
> + * memcpy(unknown, unknown, constant)  |   n   |   n   |   V   |   V   |
> + * memcpy(known,   known,   dynamic)   |   n   |   n   |   b   |   B   |
> + * memcpy(unknown, known,   dynamic)   |   n   |   n   |   V   |   B   |
> + * memcpy(known,   unknown, dynamic)   |   n   |   n   |   b   |   V   |
> + * memcpy(unknown, unknown, dynamic)   |   n   |   n   |   V   |   V   |
> + *                                     +-------+-------+-------+-------+
> + *
> + * y = deterministic compile-time bounds checking
> + * n = cannot do deterministic compile-time bounds checking
> + * n/a = no run-time bounds checking needed since compile-time deterministic
> + * b = perform run-time bounds checking
> + * B = can perform run-time bounds checking, but current unenforced
> + * V = vulnerable to run-time overflow
> + *
> + */
> +__FORTIFY_INLINE void 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)
>  {
> -       size_t p_size = __builtin_object_size(p, 0);
> -       size_t q_size = __builtin_object_size(q, 0);
> -
>         if (__builtin_constant_p(size)) {
> -               if (p_size < size)
> +               /*
> +                * Length argument is a constant expression, so we
> +                * can perform compile-time bounds checking where
> +                * buffer sizes are known.
> +                */
> +
> +               /* Error when size is larger than enclosing struct. */
> +               if (p_size > p_size_field && p_size < size)
>                         __write_overflow();
> -               if (q_size < size)
> +               if (q_size > q_size_field && q_size < size)
>                         __read_overflow2();
> +
> +               /* Warn when write size argument larger than dest field. */
> +               if (p_size_field < size)
> +                       __write_overflow_field();
> +               /*
> +                * 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) || p_size_field < size) &&
> +                   q_size_field < size)
> +                       __read_overflow2_field();
>         }
> -       if (p_size < size || q_size < size)
> -               fortify_panic(__func__);
> -       return __underlying_memcpy(p, q, 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 -1 test is to optimize away checks where the buffer
> +        * lengths are unknown.)
> +        */
> +       if ((p_size != (size_t)(-1) && p_size < size) ||
> +           (q_size != (size_t)(-1) && q_size < size))
> +               fortify_panic(func);
>  }
>
> +#define __fortify_memcpy_chk(p, q, size, p_size, q_size,               \
> +                            p_size_field, q_size_field, op) ({         \
> +       size_t __fortify_size = (size_t)(size);                         \
> +       fortify_memcpy_chk(__fortify_size, p_size, q_size,              \
> +                          p_size_field, q_size_field, #op);            \
> +       __underlying_##op(p, q, __fortify_size);                        \
> +})

Are there other macro expansion sites for `__fortify_memcpy_chk`,
perhaps later in this series? I don't understand why `memcpy` is
passed as `func` to `fortify_panic()` rather than continuing to use
`__func__`?

> +
> +/*
> + * __builtin_object_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,                 \
> +               __builtin_object_size(p, 0), __builtin_object_size(q, 0), \
> +               __builtin_object_size(p, 1), __builtin_object_size(q, 1), \
> +               memcpy)
> +
>  __FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
>  {
>         size_t p_size = __builtin_object_size(p, 0);
> @@ -277,27 +364,27 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
>         return __real_kmemdup(p, size, gfp);
>  }
>
> -/* defined after fortified strlen and memcpy to reuse them */
> +/* Defined after fortified strlen to reuse it. */
>  __FORTIFY_INLINE char *strcpy(char *p, const char *q)
>  {
>         size_t p_size = __builtin_object_size(p, 1);
>         size_t q_size = __builtin_object_size(q, 1);
>         size_t size;
>
> +       /* If neither buffer size is known, immediately give up. */
>         if (p_size == (size_t)-1 && q_size == (size_t)-1)
>                 return __underlying_strcpy(p, q);
>         size = strlen(q) + 1;
>         /* test here to use the more stringent object size */
>         if (p_size < size)
>                 fortify_panic(__func__);
> -       memcpy(p, q, size);
> +       __underlying_memcpy(p, q, size);
>         return p;
>  }
>
>  /* Don't use these outside the FORITFY_SOURCE implementation */
>  #undef __underlying_memchr
>  #undef __underlying_memcmp
> -#undef __underlying_memcpy
>  #undef __underlying_memmove
>  #undef __underlying_memset
>  #undef __underlying_strcat
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 9473f81b9db2..cbe889e404e2 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -261,8 +261,9 @@ static inline const char *kbasename(const char *path)
>   * @count: The number of bytes to copy
>   * @pad: Character to use for padding if space is left in destination.
>   */
> -static inline void memcpy_and_pad(void *dest, size_t dest_len,
> -                                 const void *src, size_t count, int pad)
> +static __always_inline void memcpy_and_pad(void *dest, size_t dest_len,
> +                                          const void *src, size_t count,
> +                                          int pad)

Why __always_inline here?

>  {
>         if (dest_len > count) {
>                 memcpy(dest, src, count);
> diff --git a/lib/Makefile b/lib/Makefile
> index 083a19336e20..74523fd394bd 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -370,7 +370,8 @@ TEST_FORTIFY_LOG = test_fortify.log
>  quiet_cmd_test_fortify = TEST    $@
>        cmd_test_fortify = $(CONFIG_SHELL) $(srctree)/scripts/test_fortify.sh \
>                         $< $@ "$(NM)" $(CC) $(c_flags) \
> -                       $(call cc-disable-warning,fortify-source)
> +                       $(call cc-disable-warning,fortify-source) \
> +                       -DKBUILD_EXTRA_WARN1
>
>  targets += $(TEST_FORTIFY_LOGS)
>  clean-files += $(TEST_FORTIFY_LOGS)
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index faa9d8e4e2c5..4d205bf5993c 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -884,6 +884,12 @@ char *strreplace(char *s, char old, char new)
>  EXPORT_SYMBOL(strreplace);
>
>  #ifdef CONFIG_FORTIFY_SOURCE
> +/* These are placeholders for fortify compile-time warnings. */
> +void __read_overflow2_field(void) { }
> +EXPORT_SYMBOL(__read_overflow2_field);
> +void __write_overflow_field(void) { }
> +EXPORT_SYMBOL(__write_overflow_field);
> +

Don't we rely on these being undefined for Clang to produce a linkage
failure (until https://reviews.llvm.org/D106030 has landed)?  By
providing a symbol definition we can link against, I don't think
__compiletime_{warning|error} will warn at all with Clang?

>  void fortify_panic(const char *name)
>  {
>         pr_emerg("detected buffer overflow in %s\n", name);
> diff --git a/lib/test_fortify/read_overflow2_field-memcpy.c b/lib/test_fortify/read_overflow2_field-memcpy.c
> new file mode 100644
> index 000000000000..de9569266223
> --- /dev/null
> +++ b/lib/test_fortify/read_overflow2_field-memcpy.c
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define TEST   \
> +       memcpy(large, instance.buf, sizeof(instance.buf) + 1)
> +
> +#include "test_fortify.h"
> diff --git a/lib/test_fortify/write_overflow_field-memcpy.c b/lib/test_fortify/write_overflow_field-memcpy.c
> new file mode 100644
> index 000000000000..28cc81058dd3
> --- /dev/null
> +++ b/lib/test_fortify/write_overflow_field-memcpy.c
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#define TEST   \
> +       memcpy(instance.buf, large, sizeof(instance.buf) + 1)
> +
> +#include "test_fortify.h"
> --

I haven't read the whole series yet, but I assume test_fortify.h was
provided earlier in the series?
-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2021-07-27 22:43 UTC|newest]

Thread overview: 294+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 20:57 [PATCH 00/64] Introduce strict memcpy() bounds checking Kees Cook
2021-07-27 20:57 ` Kees Cook
2021-07-27 20:57 ` [PATCH 01/64] media: omap3isp: Extract struct group for memcpy() region Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  0:55   ` Gustavo A. R. Silva
2021-07-28  0:55     ` Gustavo A. R. Silva
2021-07-28  1:50     ` Kees Cook
2021-07-28  1:50       ` Kees Cook
2021-07-28  8:59   ` David Sterba
2021-07-28  8:59     ` David Sterba
2021-07-28  9:14     ` Dan Carpenter
2021-07-28 21:37       ` Bart Van Assche
2021-07-28 21:37         ` David Sterba
2021-07-28 21:37           ` David Sterba
2021-07-29  5:56           ` Greg Kroah-Hartman
2021-07-29  8:20             ` Dan Carpenter
2021-07-29  8:20               ` Dan Carpenter
2021-07-30  6:00               ` Kees Cook
2021-07-30  6:00                 ` Kees Cook
2021-07-30  8:38                 ` David Sterba
2021-07-30  8:38                   ` David Sterba
2021-07-30  9:00                   ` Dan Carpenter
2021-07-30 16:44                     ` Kees Cook
2021-07-30 17:08                       ` Nick Desaulniers
2021-07-30 19:18                         ` Kees Cook
2021-07-27 20:57 ` [PATCH 02/64] mac80211: Use flex-array for radiotap header bitmap Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  7:35   ` Dan Carpenter
2021-07-28  7:35     ` Dan Carpenter
2021-07-28  9:23     ` David Sterba
2021-07-28  9:23       ` David Sterba
2021-07-28 21:54       ` Kees Cook
2021-07-29 10:45         ` David Sterba
2021-07-29 10:45           ` David Sterba
2021-07-30  6:06           ` Kees Cook
2021-07-28 21:20     ` Kees Cook
2021-07-28 21:20       ` Kees Cook
2021-07-28 23:14     ` Kees Cook
2021-07-28 23:14       ` Kees Cook
2021-07-28 23:33     ` Kees Cook
2021-07-28 23:33       ` Kees Cook
2021-07-29  8:25       ` Dan Carpenter
2021-07-29  8:25         ` Dan Carpenter
2021-07-27 20:57 ` [PATCH 03/64] rpmsg: glink: Replace strncpy() with strscpy_pad() Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  2:07   ` Gustavo A. R. Silva
2021-07-28  2:07     ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 04/64] stddef: Introduce struct_group() helper macro Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  2:32   ` Gustavo A. R. Silva
2021-07-28  2:32     ` Gustavo A. R. Silva
2021-07-28 10:54   ` Rasmus Villemoes
2021-07-28 10:54     ` Rasmus Villemoes
2021-07-28 21:59     ` Kees Cook
2021-07-28 21:59       ` Kees Cook
2021-07-30 22:19       ` Williams, Dan J
2021-07-31  2:59         ` Kees Cook
2021-07-31  5:24           ` Rasmus Villemoes
2021-07-31 15:10             ` Kees Cook
2021-07-27 20:57 ` [PATCH 05/64] skbuff: Switch structure bounds to struct_group() Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  3:50   ` Gustavo A. R. Silva
2021-07-28  3:50     ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 06/64] bnxt_en: Use struct_group_attr() for memcpy() region Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  1:03   ` Michael Chan
2021-07-28  1:03     ` Michael Chan
2021-07-28  4:45   ` Gustavo A. R. Silva
2021-07-28  4:45     ` Gustavo A. R. Silva
2021-07-27 20:57 ` [PATCH 07/64] staging: rtl8192e: Use struct_group() " Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-27 22:30   ` Larry Finger
2021-07-27 22:30     ` Larry Finger
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-28  5:45     ` Greg Kroah-Hartman
2021-07-27 20:57 ` [PATCH 08/64] staging: rtl8192u: " Kees Cook
2021-07-27 20:57   ` Kees Cook
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-28  5:45     ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 09/64] staging: rtl8723bs: Avoid field-overflowing memcpy() Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:46   ` Greg Kroah-Hartman
2021-07-28  5:46     ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 10/64] lib80211: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:52   ` Greg Kroah-Hartman
2021-07-28  5:52     ` Greg Kroah-Hartman
2021-08-13  8:04   ` Johannes Berg
2021-08-13 15:49     ` Kees Cook
2021-08-13 19:44       ` Johannes Berg
2021-07-27 20:58 ` [PATCH 11/64] net/mlx5e: Avoid field-overflowing memcpy() Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 12/64] mwl8k: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 13/64] libertas: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 14/64] libertas_tf: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 15/64] ipw2x00: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28 18:55   ` Stanislav Yakovlev
2021-07-28 18:55     ` Stanislav Yakovlev
2021-07-27 20:58 ` [PATCH 16/64] thermal: intel: int340x_thermal: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 17/64] iommu/amd: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 18/64] cxgb3: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 19/64] ip: Use struct_group() for memcpy() regions Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:55   ` Greg Kroah-Hartman
2021-07-28  5:55     ` Greg Kroah-Hartman
2021-07-28  6:14     ` Gustavo A. R. Silva
2021-07-28  6:14       ` Gustavo A. R. Silva
2021-07-28  6:19       ` Greg Kroah-Hartman
2021-07-28  6:19         ` Greg Kroah-Hartman
2021-07-28  6:31         ` Gustavo A. R. Silva
2021-07-28  6:31           ` Gustavo A. R. Silva
2021-07-28  6:37           ` Gustavo A. R. Silva
2021-07-28  6:37             ` Gustavo A. R. Silva
2021-07-28  6:41           ` Greg Kroah-Hartman
2021-07-28  6:41             ` Greg Kroah-Hartman
2021-07-28 21:01     ` Kees Cook
2021-07-28 21:01       ` Kees Cook
2021-07-29  1:59       ` Bart Van Assche
2021-07-29  1:59         ` Bart Van Assche
2021-07-27 20:58 ` [PATCH 20/64] intersil: Use struct_group() for memcpy() region Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 21/64] cxgb4: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 22/64] bnx2x: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 23/64] drm/amd/pm: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-30  2:07   ` Alex Deucher
2021-07-30  2:07     ` Alex Deucher
2021-07-27 20:58 ` [PATCH 24/64] staging: wlan-ng: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:45   ` Greg Kroah-Hartman
2021-07-28  5:45     ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 25/64] drm/mga/mga_ioc32: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:56   ` Greg Kroah-Hartman
2021-07-28  5:56     ` Greg Kroah-Hartman
2021-07-29 12:11     ` Daniel Vetter
2021-07-31  4:20       ` Kees Cook
2021-07-27 20:58 ` [PATCH 26/64] net/mlx5e: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 27/64] HID: cp2112: " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 28/64] compiler_types.h: Remove __compiletime_object_size() Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 29/64] lib/string: Move helper functions out of string.c Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 30/64] fortify: Move remaining fortify helpers into fortify-string.h Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 31/64] fortify: Explicitly disable Clang support Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 21:18   ` Nathan Chancellor
2021-07-27 21:18     ` Nathan Chancellor
2021-07-27 21:47     ` Kees Cook
2021-07-27 21:47       ` Kees Cook
2021-07-27 20:58 ` [PATCH 32/64] fortify: Add compile-time FORTIFY_SOURCE tests Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 33/64] lib: Introduce CONFIG_TEST_MEMCPY Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 23:31   ` Bart Van Assche
2021-07-27 23:31     ` Bart Van Assche
2021-07-27 23:33     ` Randy Dunlap
2021-07-27 23:33       ` Randy Dunlap
2021-07-28  1:30     ` Kees Cook
2021-07-28  1:30       ` Kees Cook
2021-07-27 20:58 ` [PATCH 34/64] fortify: Detect struct member overflows in memcpy() at compile-time Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 22:43   ` Nick Desaulniers [this message]
2021-07-27 22:43     ` Nick Desaulniers
2021-07-28  1:47     ` Kees Cook
2021-07-28  1:47       ` Kees Cook
2021-07-28  0:34   ` kernel test robot
2021-07-28  3:24   ` kernel test robot
2021-07-28 11:19   ` Rasmus Villemoes
2021-07-28 11:19     ` Rasmus Villemoes
2021-07-30  2:39     ` Kees Cook
2021-07-30  2:39       ` Kees Cook
2021-07-28 11:44   ` kernel test robot
2021-07-28 15:22   ` kernel test robot
2021-07-28 22:06   ` kernel test robot
2021-07-27 20:58 ` [PATCH 35/64] fortify: Detect struct member overflows in memmove() " Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-08-02  5:08   ` kernel test robot
2021-07-27 20:58 ` [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  1:39   ` Martin K. Petersen
2021-07-28  1:39     ` Martin K. Petersen
2021-07-28 18:57     ` Kees Cook
2021-07-28 18:57       ` Kees Cook
2021-07-29  3:35       ` Martin K. Petersen
2021-07-29  3:35         ` Martin K. Petersen
2021-07-30 19:11         ` Tyrel Datwyler
2021-07-30 18:16     ` Tyrel Datwyler
2021-07-27 20:58 ` [PATCH 37/64] string.h: Introduce memset_after() for wiping trailing members/padding Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 38/64] xfrm: Use memset_after() to clear padding Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 39/64] mac80211: Use memset_after() to clear tx status Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-31 15:55   ` Kees Cook
2021-08-13  7:40     ` Johannes Berg
2021-08-13 16:08       ` Kees Cook
2021-08-13 18:19         ` Johannes Berg
2021-08-13  7:41     ` Johannes Berg
2021-07-27 20:58 ` [PATCH 40/64] net: 802: Use memset_after() to clear struct fields Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 41/64] net: dccp: Use memset_after() for TP zeroing Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 42/64] net: qede: Use memset_after() for counters Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-31 16:07   ` Kees Cook
2021-07-27 20:58 ` [PATCH 43/64] ath11k: Use memset_after() for clearing queue descriptors Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 44/64] iw_cxgb4: Use memset_after() for cpl_t5_pass_accept_rpl Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 45/64] intel_th: msu: Use memset_after() for clearing hw header Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 46/64] IB/mthca: Use memset_after() for clearing mpt_entry Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 47/64] btrfs: Use memset_after() to clear end of struct Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  9:42   ` David Sterba
2021-07-28  9:42     ` David Sterba
2021-07-28 21:56     ` Kees Cook
2021-07-29 10:33       ` David Sterba
2021-07-29 10:33         ` David Sterba
2021-07-31 15:25         ` Kees Cook
2021-08-09 11:20           ` David Sterba
2021-07-27 20:58 ` [PATCH 48/64] drbd: Use struct_group() to zero algs Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28 21:45   ` Bart Van Assche
2021-07-28 21:45     ` Bart Van Assche
2021-07-30  2:31     ` Kees Cook
2021-07-30  2:31       ` Kees Cook
2021-07-30  2:57       ` Bart Van Assche
2021-07-30  2:57         ` Bart Van Assche
2021-07-30  9:25         ` Lars Ellenberg
2021-07-30  9:25           ` Lars Ellenberg
2021-07-30 15:32       ` Nick Desaulniers
2021-07-27 20:58 ` [PATCH 49/64] cm4000_cs: Use struct_group() to zero struct cm4000_dev region Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:48   ` Greg Kroah-Hartman
2021-07-28  5:48     ` Greg Kroah-Hartman
2021-07-27 20:58 ` [PATCH 50/64] KVM: x86: Use struct_group() to zero decode cache Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 51/64] tracing: Use struct_group() to zero struct trace_iterator Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 52/64] dm integrity: Use struct_group() to zero struct journal_sector Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 53/64] HID: roccat: Use struct_group() to zero kone_mouse_event Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 54/64] ipv6: Use struct_group() to zero rt6_info Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-29 18:58   ` Jakub Kicinski
2021-07-29 18:58     ` Jakub Kicinski
2021-07-31 15:01     ` Kees Cook
2021-07-27 20:58 ` [PATCH 55/64] RDMA/mlx5: Use struct_group() to zero struct mlx5_ib_mr Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 56/64] ethtool: stats: Use struct_group() to clear all stats at once Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 57/64] netfilter: conntrack: Use struct_group() to zero struct nf_conn Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 58/64] powerpc: Split memset() to avoid multi-field overflow Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-08-05 11:36   ` Michael Ellerman
2021-07-27 20:58 ` [PATCH 59/64] fortify: Detect struct member overflows in memset() at compile-time Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  2:59   ` kernel test robot
2021-07-28 17:18   ` kernel test robot
2021-07-27 20:58 ` [PATCH 60/64] fortify: Work around Clang inlining bugs Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 61/64] Makefile: Enable -Warray-bounds Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 62/64] netlink: Avoid false-positive memcpy() warning Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-28  5:49   ` Greg Kroah-Hartman
2021-07-28  5:49     ` Greg Kroah-Hartman
2021-07-28 11:24     ` Rasmus Villemoes
2021-07-28 11:24       ` Rasmus Villemoes
2021-07-30  1:39       ` Kees Cook
2021-07-30  1:39         ` Kees Cook
2021-07-30  1:41     ` Kees Cook
2021-07-30  1:41       ` Kees Cook
2021-07-27 20:58 ` [PATCH 63/64] iwlwifi: dbg_ini: Split memcpy() to avoid multi-field write Kees Cook
2021-07-27 20:58   ` Kees Cook
2021-07-27 20:58 ` [PATCH 64/64] fortify: Add run-time WARN for cross-field memcpy() Kees Cook
2021-07-27 20:58   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKwvOdknit8DtWaFvLupmNEebjbwVa6R3xiGc2D4AqB_6+i52g@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=gustavoars@kernel.org \
    --cc=keescook@chromium.org \
    --cc=keithpac@amazon.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.