[6/9] fortify: Split reporting and avoid passing string pointer
Commit Message
In preparation for KUnit testing and further improvements in fortify
failure reporting, split out the report and encode the function and
access failure (read or write overflow) into a single int argument. This
mainly ends up saving some space in the data segment. For a defconfig
with FORTIFY_SOURCE enabled:
$ size gcc/vmlinux.before gcc/vmlinux.after
text data bss dec hex filename
26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Cezary Rojewski <cezary.rojewski@intel.com>
Cc: Puyou Lu <puyou.lu@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/fortify-string.h | 72 +++++++++++++++++++++++-----------
lib/string_helpers.c | 70 +++++++++++++++++++++++++++++++--
tools/objtool/check.c | 2 +-
3 files changed, 118 insertions(+), 26 deletions(-)
Comments
On Thu, Apr 6, 2023 at 3:02 AM Kees Cook <keescook@chromium.org> wrote:
>
> In preparation for KUnit testing and further improvements in fortify
> failure reporting, split out the report and encode the function and
> access failure (read or write overflow) into a single int argument. This
> mainly ends up saving some space in the data segment. For a defconfig
> with FORTIFY_SOURCE enabled:
>
> $ size gcc/vmlinux.before gcc/vmlinux.after
> text data bss dec hex filename
> 26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
> 26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
...
> + const char *name;
> + const bool write = !!(reason & 0x1);
Perhaps define that as
FORTIFY_READ_WRITE BIT(0)
FORTIFY_FUNC_SHIFT 1
const bool write = reason & FORTIFY_READ_WRITE; // and note no need for !! part
switch (reason >> FORTIFY_FUNC_SHIFT) {
> + switch (reason >> 1) {
> + case FORTIFY_FUNC_strncpy:
> + name = "strncpy";
> + break;
> + case FORTIFY_FUNC_strnlen:
> + name = "strnlen";
> + break;
> + case FORTIFY_FUNC_strlen:
> + name = "strlen";
> + break;
> + case FORTIFY_FUNC_strlcpy:
> + name = "strlcpy";
> + break;
> + case FORTIFY_FUNC_strscpy:
> + name = "strscpy";
> + break;
> + case FORTIFY_FUNC_strlcat:
> + name = "strlcat";
> + break;
> + case FORTIFY_FUNC_strcat:
> + name = "strcat";
> + break;
> + case FORTIFY_FUNC_strncat:
> + name = "strncat";
> + break;
> + case FORTIFY_FUNC_memset:
> + name = "memset";
> + break;
> + case FORTIFY_FUNC_memcpy:
> + name = "memcpy";
> + break;
> + case FORTIFY_FUNC_memmove:
> + name = "memmove";
> + break;
> + case FORTIFY_FUNC_memscan:
> + name = "memscan";
> + break;
> + case FORTIFY_FUNC_memcmp:
> + name = "memcmp";
> + break;
> + case FORTIFY_FUNC_memchr:
> + name = "memchr";
> + break;
> + case FORTIFY_FUNC_memchr_inv:
> + name = "memchr_inv";
> + break;
> + case FORTIFY_FUNC_kmemdup:
> + name = "kmemdup";
> + break;
> + case FORTIFY_FUNC_strcpy:
> + name = "strcpy";
> + break;
> + default:
> + name = "unknown";
> + }
...
> + WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
Using str_read_write() ?
Dunno if it's already there or needs to be added. I have some patches
to move those str_*() to string_choices.h. We can also prepend yours
with those.
On Thu, Apr 6, 2023 at 2:02 AM Kees Cook <keescook@chromium.org> wrote:
>
> +void __fortify_report(u8 reason);
> +void __fortify_panic(u8 reason) __cold __noreturn;
(snip)
> +void __fortify_report(u8 reason)
(snip)
> +void __fortify_panic(const u8 reason)
I am curious: for some reason (no pun intended :) the `reason`s above
are not `const` except this one, but then in a later patch they become
`const` (including the declarations).
So perhaps make everything `const` when they are introduced? Or is
there some other reason? (e.g. I saw one patch that moved a function,
so there it seemed to make sense to keep things as they are to make
the copy 1:1).
Cheers,
Miguel
From: Kees Cook <keescook@chromium.org>
Date: Wed, 5 Apr 2023 17:02:05 -0700
> In preparation for KUnit testing and further improvements in fortify
> failure reporting, split out the report and encode the function and
> access failure (read or write overflow) into a single int argument. This
> mainly ends up saving some space in the data segment. For a defconfig
> with FORTIFY_SOURCE enabled:
>
> $ size gcc/vmlinux.before gcc/vmlinux.after
> text data bss dec hex filename
> 26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
> 26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
>
> Cc: Andy Shevchenko <andy@kernel.org>
> Cc: Cezary Rojewski <cezary.rojewski@intel.com>
> Cc: Puyou Lu <puyou.lu@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-hardening@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> include/linux/fortify-string.h | 72 +++++++++++++++++++++++-----------
> lib/string_helpers.c | 70 +++++++++++++++++++++++++++++++--
> tools/objtool/check.c | 2 +-
> 3 files changed, 118 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> index 41dbd641f55c..6db4052db459 100644
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -9,7 +9,34 @@
> #define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
> #define __RENAME(x) __asm__(#x)
>
> -void fortify_panic(const char *name) __noreturn __cold;
> +#define fortify_reason(func, write) (((func) << 1) | !!(write))
> +
> +#define fortify_panic(func, write) \
> + __fortify_panic(fortify_reason(func, write))
> +
> +#define FORTIFY_READ 0
> +#define FORTIFY_WRITE 1
> +
> +#define FORTIFY_FUNC_strncpy 0
> +#define FORTIFY_FUNC_strnlen 1
> +#define FORTIFY_FUNC_strlen 2
> +#define FORTIFY_FUNC_strlcpy 3
> +#define FORTIFY_FUNC_strscpy 4
> +#define FORTIFY_FUNC_strlcat 5
> +#define FORTIFY_FUNC_strcat 6
> +#define FORTIFY_FUNC_strncat 7
> +#define FORTIFY_FUNC_memset 8
> +#define FORTIFY_FUNC_memcpy 9
> +#define FORTIFY_FUNC_memmove 10
> +#define FORTIFY_FUNC_memscan 11
> +#define FORTIFY_FUNC_memcmp 12
> +#define FORTIFY_FUNC_memchr 13
> +#define FORTIFY_FUNC_memchr_inv 14
> +#define FORTIFY_FUNC_kmemdup 15
> +#define FORTIFY_FUNC_strcpy 16
enum?
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -1021,10 +1021,74 @@ EXPORT_SYMBOL(__read_overflow2_field);
> void __write_overflow_field(size_t avail, size_t wanted) { }
> EXPORT_SYMBOL(__write_overflow_field);
>
> -void fortify_panic(const char *name)
> +void __fortify_report(u8 reason)
> {
> - pr_emerg("detected buffer overflow in %s\n", name);
> + const char *name;
> + const bool write = !!(reason & 0x1);
> +
> + switch (reason >> 1) {
As already mentioned, I'd use bitfield helpers + couple definitions to
not miss something when changing the way it's encoded
#define FORTIFY_REASON_DIR(r) FIELD_GET(BIT(0), r)
#define FORTIFY_REASON_FUNC(r) FIELD_GET(GENMASK(7, 1), r)
(+ set pair)
> + case FORTIFY_FUNC_strncpy:
> + name = "strncpy";
> + break;
> + case FORTIFY_FUNC_strnlen:
> + name = "strnlen";
> + break;
> + case FORTIFY_FUNC_strlen:
> + name = "strlen";
> + break;
> + case FORTIFY_FUNC_strlcpy:
> + name = "strlcpy";
> + break;
> + case FORTIFY_FUNC_strscpy:
> + name = "strscpy";
> + break;
> + case FORTIFY_FUNC_strlcat:
> + name = "strlcat";
> + break;
> + case FORTIFY_FUNC_strcat:
> + name = "strcat";
> + break;
> + case FORTIFY_FUNC_strncat:
> + name = "strncat";
> + break;
> + case FORTIFY_FUNC_memset:
> + name = "memset";
> + break;
> + case FORTIFY_FUNC_memcpy:
> + name = "memcpy";
> + break;
> + case FORTIFY_FUNC_memmove:
> + name = "memmove";
> + break;
> + case FORTIFY_FUNC_memscan:
> + name = "memscan";
> + break;
> + case FORTIFY_FUNC_memcmp:
> + name = "memcmp";
> + break;
> + case FORTIFY_FUNC_memchr:
> + name = "memchr";
> + break;
> + case FORTIFY_FUNC_memchr_inv:
> + name = "memchr_inv";
> + break;
> + case FORTIFY_FUNC_kmemdup:
> + name = "kmemdup";
> + break;
> + case FORTIFY_FUNC_strcpy:
> + name = "strcpy";
> + break;
> + default:
> + name = "unknown";
> + }
I know this is far from hotpath, but could we save some object code and
do that via O(1) array lookup? Plus some macro to compress things:
#define FORTIFY_ENTRY(name) \
[FORTIFY_FUNC_##name] = #name
static const char * const fortify_funcs[] = {
FORTIFY_ENTRY(strncpy),
...
}
// array bounds check here if you wish, I wouldn't bother as
// we're panicking anyway
name = fortify_funcs[reason >> 1];
> + WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
> +}
> +EXPORT_SYMBOL(__fortify_report);
> +
> +void __fortify_panic(const u8 reason)
> +{
> + __fortify_report(reason);
> BUG();
> }
> -EXPORT_SYMBOL(fortify_panic);
> +EXPORT_SYMBOL(__fortify_panic);
> #endif /* CONFIG_FORTIFY_SOURCE */
Thanks,
Olek
On Thu, Apr 06, 2023 at 05:23:54PM +0200, Alexander Lobakin wrote:
> From: Kees Cook <keescook@chromium.org>
> Date: Wed, 5 Apr 2023 17:02:05 -0700
>
> > In preparation for KUnit testing and further improvements in fortify
> > failure reporting, split out the report and encode the function and
> > access failure (read or write overflow) into a single int argument. This
> > mainly ends up saving some space in the data segment. For a defconfig
> > with FORTIFY_SOURCE enabled:
> >
> > $ size gcc/vmlinux.before gcc/vmlinux.after
> > text data bss dec hex filename
> > 26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
> > 26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
> >
> > Cc: Andy Shevchenko <andy@kernel.org>
> > Cc: Cezary Rojewski <cezary.rojewski@intel.com>
> > Cc: Puyou Lu <puyou.lu@gmail.com>
> > Cc: Mark Brown <broonie@kernel.org>
> > Cc: linux-hardening@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > include/linux/fortify-string.h | 72 +++++++++++++++++++++++-----------
> > lib/string_helpers.c | 70 +++++++++++++++++++++++++++++++--
> > tools/objtool/check.c | 2 +-
> > 3 files changed, 118 insertions(+), 26 deletions(-)
> >
> > diff --git a/include/linux/fortify-string.h b/include/linux/fortify-string.h
> > index 41dbd641f55c..6db4052db459 100644
> > --- a/include/linux/fortify-string.h
> > +++ b/include/linux/fortify-string.h
> > @@ -9,7 +9,34 @@
> > #define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
> > #define __RENAME(x) __asm__(#x)
> >
> > -void fortify_panic(const char *name) __noreturn __cold;
> > +#define fortify_reason(func, write) (((func) << 1) | !!(write))
> > +
> > +#define fortify_panic(func, write) \
> > + __fortify_panic(fortify_reason(func, write))
> > +
> > +#define FORTIFY_READ 0
> > +#define FORTIFY_WRITE 1
> > +
> > +#define FORTIFY_FUNC_strncpy 0
> > +#define FORTIFY_FUNC_strnlen 1
> > +#define FORTIFY_FUNC_strlen 2
> > +#define FORTIFY_FUNC_strlcpy 3
> > +#define FORTIFY_FUNC_strscpy 4
> > +#define FORTIFY_FUNC_strlcat 5
> > +#define FORTIFY_FUNC_strcat 6
> > +#define FORTIFY_FUNC_strncat 7
> > +#define FORTIFY_FUNC_memset 8
> > +#define FORTIFY_FUNC_memcpy 9
> > +#define FORTIFY_FUNC_memmove 10
> > +#define FORTIFY_FUNC_memscan 11
> > +#define FORTIFY_FUNC_memcmp 12
> > +#define FORTIFY_FUNC_memchr 13
> > +#define FORTIFY_FUNC_memchr_inv 14
> > +#define FORTIFY_FUNC_kmemdup 15
> > +#define FORTIFY_FUNC_strcpy 16
>
> enum?
I opted to avoid an enum due to the binary operations used in
"fortify_reason" to collapse it to a u8. It just seemed like more work
to put in enums, and then do u8 casts, etc, all for a strictly
"internal" set of magic numbers.
>
> > --- a/lib/string_helpers.c
> > +++ b/lib/string_helpers.c
> > @@ -1021,10 +1021,74 @@ EXPORT_SYMBOL(__read_overflow2_field);
> > void __write_overflow_field(size_t avail, size_t wanted) { }
> > EXPORT_SYMBOL(__write_overflow_field);
> >
> > -void fortify_panic(const char *name)
> > +void __fortify_report(u8 reason)
> > {
> > - pr_emerg("detected buffer overflow in %s\n", name);
> > + const char *name;
> > + const bool write = !!(reason & 0x1);
> > +
> > + switch (reason >> 1) {
>
> As already mentioned, I'd use bitfield helpers + couple definitions to
> not miss something when changing the way it's encoded
>
> #define FORTIFY_REASON_DIR(r) FIELD_GET(BIT(0), r)
> #define FORTIFY_REASON_FUNC(r) FIELD_GET(GENMASK(7, 1), r)
Yeah, good idea. Thanks for the FIELD_GET examples!
> [...]
> > + break;
> > + default:
> > + name = "unknown";
> > + }
>
> I know this is far from hotpath, but could we save some object code and
> do that via O(1) array lookup? Plus some macro to compress things:
>
> #define FORTIFY_ENTRY(name) \
> [FORTIFY_FUNC_##name] = #name
>
> static const char * const fortify_funcs[] = {
> FORTIFY_ENTRY(strncpy),
> ...
> }
>
> // array bounds check here if you wish, I wouldn't bother as
> // we're panicking anyway
>
> name = fortify_funcs[reason >> 1];
Fair enough. I had considered it and then forgot about it for some
reason. :)
On Thu, Apr 06, 2023 at 03:44:54PM +0200, Miguel Ojeda wrote:
> On Thu, Apr 6, 2023 at 2:02 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > +void __fortify_report(u8 reason);
> > +void __fortify_panic(u8 reason) __cold __noreturn;
>
> (snip)
>
> > +void __fortify_report(u8 reason)
>
> (snip)
>
> > +void __fortify_panic(const u8 reason)
>
> I am curious: for some reason (no pun intended :) the `reason`s above
> are not `const` except this one, but then in a later patch they become
> `const` (including the declarations).
>
> So perhaps make everything `const` when they are introduced? Or is
> there some other reason? (e.g. I saw one patch that moved a function,
> so there it seemed to make sense to keep things as they are to make
> the copy 1:1).
I will adjust it -- this was an artifact of splitting up my patches.
Thanks!
On Thu, Apr 06, 2023 at 01:20:52PM +0300, Andy Shevchenko wrote:
> On Thu, Apr 6, 2023 at 3:02 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > In preparation for KUnit testing and further improvements in fortify
> > failure reporting, split out the report and encode the function and
> > access failure (read or write overflow) into a single int argument. This
> > mainly ends up saving some space in the data segment. For a defconfig
> > with FORTIFY_SOURCE enabled:
> >
> > $ size gcc/vmlinux.before gcc/vmlinux.after
> > text data bss dec hex filename
> > 26132309 9760658 2195460 38088427 2452eeb gcc/vmlinux.before
> > 26132386 9748382 2195460 38076228 244ff44 gcc/vmlinux.after
>
> ...
>
> > + const char *name;
> > + const bool write = !!(reason & 0x1);
>
> Perhaps define that as
>
> FORTIFY_READ_WRITE BIT(0)
> FORTIFY_FUNC_SHIFT 1
>
> const bool write = reason & FORTIFY_READ_WRITE; // and note no need for !! part
Yeah, that reads better. The FIELD_GET suggestion down-thread is
probably how I'll go.
>
> switch (reason >> FORTIFY_FUNC_SHIFT) {
>
> > + switch (reason >> 1) {
> > + case FORTIFY_FUNC_strncpy:
> > + name = "strncpy";
> > + break;
> > + case FORTIFY_FUNC_strnlen:
> > + name = "strnlen";
> > + break;
> > + case FORTIFY_FUNC_strlen:
> > + name = "strlen";
> > + break;
> > + case FORTIFY_FUNC_strlcpy:
> > + name = "strlcpy";
> > + break;
> > + case FORTIFY_FUNC_strscpy:
> > + name = "strscpy";
> > + break;
> > + case FORTIFY_FUNC_strlcat:
> > + name = "strlcat";
> > + break;
> > + case FORTIFY_FUNC_strcat:
> > + name = "strcat";
> > + break;
> > + case FORTIFY_FUNC_strncat:
> > + name = "strncat";
> > + break;
> > + case FORTIFY_FUNC_memset:
> > + name = "memset";
> > + break;
> > + case FORTIFY_FUNC_memcpy:
> > + name = "memcpy";
> > + break;
> > + case FORTIFY_FUNC_memmove:
> > + name = "memmove";
> > + break;
> > + case FORTIFY_FUNC_memscan:
> > + name = "memscan";
> > + break;
> > + case FORTIFY_FUNC_memcmp:
> > + name = "memcmp";
> > + break;
> > + case FORTIFY_FUNC_memchr:
> > + name = "memchr";
> > + break;
> > + case FORTIFY_FUNC_memchr_inv:
> > + name = "memchr_inv";
> > + break;
> > + case FORTIFY_FUNC_kmemdup:
> > + name = "kmemdup";
> > + break;
> > + case FORTIFY_FUNC_strcpy:
> > + name = "strcpy";
> > + break;
> > + default:
> > + name = "unknown";
> > + }
>
> ...
>
> > + WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
>
> Using str_read_write() ?
>
> Dunno if it's already there or needs to be added. I have some patches
> to move those str_*() to string_choices.h. We can also prepend yours
> with those.
Oh! Hah. I totally forgot about str_read_write. :) I will use that.
On Fri, Apr 7, 2023 at 1:57 AM Kees Cook <keescook@chromium.org> wrote:
> On Thu, Apr 06, 2023 at 01:20:52PM +0300, Andy Shevchenko wrote:
> > On Thu, Apr 6, 2023 at 3:02 AM Kees Cook <keescook@chromium.org> wrote:
...
> > > + WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
> >
> > Using str_read_write() ?
> >
> > Dunno if it's already there or needs to be added. I have some patches
> > to move those str_*() to string_choices.h. We can also prepend yours
> > with those.
>
> Oh! Hah. I totally forgot about str_read_write. :) I will use that.
Btw, makes sense to add
#define str_write_read(v) str_read_write(!(v))
to the header, so we won't use negation in the parameter for better readability.
On April 7, 2023 1:34:41 AM PDT, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>On Fri, Apr 7, 2023 at 1:57 AM Kees Cook <keescook@chromium.org> wrote:
>> On Thu, Apr 06, 2023 at 01:20:52PM +0300, Andy Shevchenko wrote:
>> > On Thu, Apr 6, 2023 at 3:02 AM Kees Cook <keescook@chromium.org> wrote:
>
>...
>
>> > > + WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
>> >
>> > Using str_read_write() ?
>> >
>> > Dunno if it's already there or needs to be added. I have some patches
>> > to move those str_*() to string_choices.h. We can also prepend yours
>> > with those.
>>
>> Oh! Hah. I totally forgot about str_read_write. :) I will use that.
>
>Btw, makes sense to add
>
> #define str_write_read(v) str_read_write(!(v))
>
>to the header, so we won't use negation in the parameter for better readability.
I ended up not going this far because the use of str_read_write() gets removed again in the last patch in the series.
@@ -9,7 +9,34 @@
#define __FORTIFY_INLINE extern __always_inline __gnu_inline __overloadable
#define __RENAME(x) __asm__(#x)
-void fortify_panic(const char *name) __noreturn __cold;
+#define fortify_reason(func, write) (((func) << 1) | !!(write))
+
+#define fortify_panic(func, write) \
+ __fortify_panic(fortify_reason(func, write))
+
+#define FORTIFY_READ 0
+#define FORTIFY_WRITE 1
+
+#define FORTIFY_FUNC_strncpy 0
+#define FORTIFY_FUNC_strnlen 1
+#define FORTIFY_FUNC_strlen 2
+#define FORTIFY_FUNC_strlcpy 3
+#define FORTIFY_FUNC_strscpy 4
+#define FORTIFY_FUNC_strlcat 5
+#define FORTIFY_FUNC_strcat 6
+#define FORTIFY_FUNC_strncat 7
+#define FORTIFY_FUNC_memset 8
+#define FORTIFY_FUNC_memcpy 9
+#define FORTIFY_FUNC_memmove 10
+#define FORTIFY_FUNC_memscan 11
+#define FORTIFY_FUNC_memcmp 12
+#define FORTIFY_FUNC_memchr 13
+#define FORTIFY_FUNC_memchr_inv 14
+#define FORTIFY_FUNC_kmemdup 15
+#define FORTIFY_FUNC_strcpy 16
+
+void __fortify_report(u8 reason);
+void __fortify_panic(u8 reason) __cold __noreturn;
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()?");
@@ -147,7 +174,7 @@ char *strncpy(char * const POS p, const char *q, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__write_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strncpy, FORTIFY_WRITE);
return __underlying_strncpy(p, q, size);
}
@@ -178,7 +205,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char * const POS p, __kernel_size
/* Do not check characters beyond the end of p. */
ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
if (p_size <= ret && maxlen != ret)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strnlen, FORTIFY_READ);
return ret;
}
@@ -214,7 +241,7 @@ __kernel_size_t __fortify_strlen(const char * const POS p)
return __underlying_strlen(p);
ret = strnlen(p, p_size);
if (p_size <= ret)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlen, FORTIFY_READ);
return ret;
}
@@ -256,7 +283,7 @@ __FORTIFY_INLINE size_t strlcpy(char * const POS p, const char * const POS q, si
}
if (size) {
if (len >= p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcpy, FORTIFY_WRITE);
__underlying_memcpy(p, q, len);
p[len] = '\0';
}
@@ -334,7 +361,7 @@ __FORTIFY_INLINE ssize_t strscpy(char * const POS p, const char * const POS q, s
* p_size.
*/
if (len > p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strscpy, FORTIFY_WRITE);
/*
* We can now safely call vanilla strscpy because we are protected from:
@@ -392,7 +419,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if string is already overflowed. */
if (p_size <= p_len)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_READ);
if (actual >= avail) {
copy_len = avail - p_len - 1;
@@ -401,7 +428,7 @@ size_t strlcat(char * const POS p, const char * const POS q, size_t avail)
/* Give up if copy will overflow. */
if (p_size <= actual)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strlcat, FORTIFY_WRITE);
__underlying_memcpy(p + p_len, q, copy_len);
p[actual] = '\0';
@@ -430,7 +457,7 @@ char *strcat(char * const POS p, const char *q)
size_t p_size = __member_size(p);
if (strlcat(p, q, p_size) >= p_size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strcat, FORTIFY_WRITE);
return p;
}
@@ -466,7 +493,7 @@ char *strncat(char * const POS p, const char * const POS q, __kernel_size_t coun
p_len = strlen(p);
copy_len = strnlen(q, count);
if (p_size < p_len + copy_len + 1)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strncat, FORTIFY_WRITE);
__underlying_memcpy(p + p_len, q, copy_len);
p[p_len + copy_len] = '\0';
return p;
@@ -507,7 +534,7 @@ __FORTIFY_INLINE void fortify_memset_chk(__kernel_size_t size,
* lengths are unknown.)
*/
if (p_size != SIZE_MAX && p_size < size)
- fortify_panic("memset");
+ fortify_panic(FORTIFY_FUNC_memset, FORTIFY_WRITE);
}
#define __fortify_memset_chk(p, c, size, p_size, p_size_field) ({ \
@@ -561,7 +588,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
const size_t q_size,
const size_t p_size_field,
const size_t q_size_field,
- const char *func)
+ const u8 func)
{
if (__builtin_constant_p(size)) {
/*
@@ -605,9 +632,10 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
* (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);
+ if (p_size != SIZE_MAX && p_size < size)
+ fortify_panic(func, FORTIFY_WRITE);
+ else if (q_size != SIZE_MAX && q_size < size)
+ fortify_panic(func, FORTIFY_READ);
/*
* Warn when writing beyond destination field size.
@@ -640,7 +668,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
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), \
+ __q_size_field, FORTIFY_FUNC_ ##op), \
#op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \
__fortify_size, \
"field \"" #p "\" at " __FILE__ ":" __stringify(__LINE__), \
@@ -707,7 +735,7 @@ __FORTIFY_INLINE void *memscan(void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memscan, FORTIFY_READ);
return __real_memscan(p, c, size);
}
@@ -724,7 +752,7 @@ int memcmp(const void * const POS0 p, const void * const POS0 q, __kernel_size_t
__read_overflow2();
}
if (p_size < size || q_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memcmp, FORTIFY_READ);
return __underlying_memcmp(p, q, size);
}
@@ -736,7 +764,7 @@ void *memchr(const void * const POS0 p, int c, __kernel_size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memchr, FORTIFY_READ);
return __underlying_memchr(p, c, size);
}
@@ -748,7 +776,7 @@ __FORTIFY_INLINE void *memchr_inv(const void * const POS0 p, int c, size_t size)
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_memchr_inv, FORTIFY_READ);
return __real_memchr_inv(p, c, size);
}
@@ -761,7 +789,7 @@ __FORTIFY_INLINE void *kmemdup(const void * const POS0 p, size_t size, gfp_t gfp
if (__compiletime_lessthan(p_size, size))
__read_overflow();
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_kmemdup, FORTIFY_READ);
return __real_kmemdup(p, size, gfp);
}
@@ -798,7 +826,7 @@ char *strcpy(char * const POS p, const char * const POS q)
__write_overflow();
/* Run-time check for dynamic size overflow. */
if (p_size < size)
- fortify_panic(__func__);
+ fortify_panic(FORTIFY_FUNC_strcpy, FORTIFY_WRITE);
__underlying_memcpy(p, q, size);
return p;
}
@@ -1021,10 +1021,74 @@ EXPORT_SYMBOL(__read_overflow2_field);
void __write_overflow_field(size_t avail, size_t wanted) { }
EXPORT_SYMBOL(__write_overflow_field);
-void fortify_panic(const char *name)
+void __fortify_report(u8 reason)
{
- pr_emerg("detected buffer overflow in %s\n", name);
+ const char *name;
+ const bool write = !!(reason & 0x1);
+
+ switch (reason >> 1) {
+ case FORTIFY_FUNC_strncpy:
+ name = "strncpy";
+ break;
+ case FORTIFY_FUNC_strnlen:
+ name = "strnlen";
+ break;
+ case FORTIFY_FUNC_strlen:
+ name = "strlen";
+ break;
+ case FORTIFY_FUNC_strlcpy:
+ name = "strlcpy";
+ break;
+ case FORTIFY_FUNC_strscpy:
+ name = "strscpy";
+ break;
+ case FORTIFY_FUNC_strlcat:
+ name = "strlcat";
+ break;
+ case FORTIFY_FUNC_strcat:
+ name = "strcat";
+ break;
+ case FORTIFY_FUNC_strncat:
+ name = "strncat";
+ break;
+ case FORTIFY_FUNC_memset:
+ name = "memset";
+ break;
+ case FORTIFY_FUNC_memcpy:
+ name = "memcpy";
+ break;
+ case FORTIFY_FUNC_memmove:
+ name = "memmove";
+ break;
+ case FORTIFY_FUNC_memscan:
+ name = "memscan";
+ break;
+ case FORTIFY_FUNC_memcmp:
+ name = "memcmp";
+ break;
+ case FORTIFY_FUNC_memchr:
+ name = "memchr";
+ break;
+ case FORTIFY_FUNC_memchr_inv:
+ name = "memchr_inv";
+ break;
+ case FORTIFY_FUNC_kmemdup:
+ name = "kmemdup";
+ break;
+ case FORTIFY_FUNC_strcpy:
+ name = "strcpy";
+ break;
+ default:
+ name = "unknown";
+ }
+ WARN(1, "%s: detected buffer %s overflow\n", name, write ? "write" : "read");
+}
+EXPORT_SYMBOL(__fortify_report);
+
+void __fortify_panic(const u8 reason)
+{
+ __fortify_report(reason);
BUG();
}
-EXPORT_SYMBOL(fortify_panic);
+EXPORT_SYMBOL(__fortify_panic);
#endif /* CONFIG_FORTIFY_SOURCE */
@@ -197,6 +197,7 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
* attribute isn't provided in ELF data. Keep 'em sorted.
*/
static const char * const global_noreturns[] = {
+ "__fortify_panic",
"__invalid_creds",
"__module_put_and_kthread_exit",
"__reiserfs_panic",
@@ -208,7 +209,6 @@ static bool __dead_end_function(struct objtool_file *file, struct symbol *func,
"do_group_exit",
"do_task_dead",
"ex_handler_msr_mce",
- "fortify_panic",
"kthread_complete_and_exit",
"kthread_exit",
"kunit_try_catch_throw",