Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1697485 > unrolled thread

[PATCH v2] fortify: Use WARN instead of BUG for now

Started byKees Cook <keescook@chromium.org>
First post2017-07-26 21:00 +0200
Last post2017-07-29 11:00 +0200
Articles 4 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2] fortify: Use WARN instead of BUG for now Kees Cook <keescook@chromium.org> - 2017-07-26 21:00 +0200
    Re: [PATCH v2] fortify: Use WARN instead of BUG for now Josh Poimboeuf <jpoimboe@redhat.com> - 2017-07-26 22:00 +0200
      Re: [PATCH v2] fortify: Use WARN instead of BUG for now Kees Cook <keescook@chromium.org> - 2017-07-26 23:40 +0200
    Re: [PATCH v2] fortify: Use WARN instead of BUG for now kbuild test robot <lkp@intel.com> - 2017-07-29 11:00 +0200

#1697485 — [PATCH v2] fortify: Use WARN instead of BUG for now

FromKees Cook <keescook@chromium.org>
Date2017-07-26 21:00 +0200
Subject[PATCH v2] fortify: Use WARN instead of BUG for now
Message-ID<u7zHk-3ZT-15@gated-at.bofh.it>
While CONFIG_FORTIFY_SOURCE continues to shake out, don't unconditionally
use BUG(), opting instead for WARN(). This also renames fortify_panic()
to fortify_overflow() which better matches what it is being called for.

Cc: Daniel Micay <danielmicay@gmail.com>
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
- just do simple renaming, no logic changes (danielmicay)

Sending to akpm, since fortify went through -mm originally.
---
 arch/x86/boot/compressed/misc.c |  2 +-
 include/linux/string.h          | 30 +++++++++++++++---------------
 lib/string.c                    |  7 +++----
 tools/objtool/check.c           |  2 +-
 4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index a0838ab929f2..c20cdc7cbd61 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -412,7 +412,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap,
 	return output;
 }
 
-void fortify_panic(const char *name)
+void fortify_overflow(const char *name)
 {
 	error("detected buffer overflow");
 }
diff --git a/include/linux/string.h b/include/linux/string.h
index a467e617eeb0..25f47e07c4c6 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -197,7 +197,7 @@ static inline const char *kbasename(const char *path)
 #define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline))
 #define __RENAME(x) __asm__(#x)
 
-void fortify_panic(const char *name) __noreturn __cold;
+void fortify_overflow(const char *name) __cold;
 void __read_overflow(void) __compiletime_error("detected read beyond size of object passed as 1st parameter");
 void __read_overflow2(void) __compiletime_error("detected read beyond size of object passed as 2nd parameter");
 void __write_overflow(void) __compiletime_error("detected write beyond size of object passed as 1st parameter");
@@ -209,7 +209,7 @@ __FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
 	if (__builtin_constant_p(size) && p_size < size)
 		__write_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_strncpy(p, q, size);
 }
 
@@ -219,7 +219,7 @@ __FORTIFY_INLINE char *strcat(char *p, const char *q)
 	if (p_size == (size_t)-1)
 		return __builtin_strcat(p, q);
 	if (strlcat(p, q, p_size) >= p_size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return p;
 }
 
@@ -231,7 +231,7 @@ __FORTIFY_INLINE __kernel_size_t strlen(const char *p)
 		return __builtin_strlen(p);
 	ret = strnlen(p, p_size);
 	if (p_size <= ret)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return ret;
 }
 
@@ -241,7 +241,7 @@ __FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen)
 	size_t p_size = __builtin_object_size(p, 0);
 	__kernel_size_t ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size);
 	if (p_size <= ret && maxlen != ret)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return ret;
 }
 
@@ -260,7 +260,7 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size)
 		if (__builtin_constant_p(len) && len >= p_size)
 			__write_overflow();
 		if (len >= p_size)
-			fortify_panic(__func__);
+			fortify_overflow(__func__);
 		__builtin_memcpy(p, q, len);
 		p[len] = '\0';
 	}
@@ -278,7 +278,7 @@ __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count)
 	p_len = strlen(p);
 	copy_len = strnlen(q, count);
 	if (p_size < p_len + copy_len + 1)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	__builtin_memcpy(p + p_len, q, copy_len);
 	p[p_len + copy_len] = '\0';
 	return p;
@@ -290,7 +290,7 @@ __FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size)
 	if (__builtin_constant_p(size) && p_size < size)
 		__write_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_memset(p, c, size);
 }
 
@@ -305,7 +305,7 @@ __FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
 			__read_overflow2();
 	}
 	if (p_size < size || q_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_memcpy(p, q, size);
 }
 
@@ -320,7 +320,7 @@ __FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size)
 			__read_overflow2();
 	}
 	if (p_size < size || q_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_memmove(p, q, size);
 }
 
@@ -331,7 +331,7 @@ __FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size)
 	if (__builtin_constant_p(size) && p_size < size)
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __real_memscan(p, c, size);
 }
 
@@ -346,7 +346,7 @@ __FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size)
 			__read_overflow2();
 	}
 	if (p_size < size || q_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_memcmp(p, q, size);
 }
 
@@ -356,7 +356,7 @@ __FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size)
 	if (__builtin_constant_p(size) && p_size < size)
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __builtin_memchr(p, c, size);
 }
 
@@ -367,7 +367,7 @@ __FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size)
 	if (__builtin_constant_p(size) && p_size < size)
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __real_memchr_inv(p, c, size);
 }
 
@@ -378,7 +378,7 @@ __FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp)
 	if (__builtin_constant_p(size) && p_size < size)
 		__read_overflow();
 	if (p_size < size)
-		fortify_panic(__func__);
+		fortify_overflow(__func__);
 	return __real_kmemdup(p, size, gfp);
 }
 
diff --git a/lib/string.c b/lib/string.c
index ebbb99c775bd..e8fc0c495442 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -979,9 +979,8 @@ char *strreplace(char *s, char old, char new)
 }
 EXPORT_SYMBOL(strreplace);
 
-void fortify_panic(const char *name)
+void fortify_overflow(const char *name)
 {
-	pr_emerg("detected buffer overflow in %s\n", name);
-	BUG();
+	WARN(1, "detected buffer overflow in %s\n", name);
 }
-EXPORT_SYMBOL(fortify_panic);
+EXPORT_SYMBOL(fortify_overflow);
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 2c6d74880403..9e45de4d2e72 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -156,7 +156,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
 		"kvm_spurious_fault",
 		"__reiserfs_panic",
 		"lbug_with_loc",
-		"fortify_panic",
+		"fortify_overflow",
 	};
 
 	if (func->bind == STB_WEAK)
-- 
2.7.4


-- 
Kees Cook
Pixel Security

[toc] | [next] | [standalone]


#1697529

FromJosh Poimboeuf <jpoimboe@redhat.com>
Date2017-07-26 22:00 +0200
Message-ID<u7ADo-4Bn-27@gated-at.bofh.it>
In reply to#1697485
On Wed, Jul 26, 2017 at 11:52:19AM -0700, Kees Cook wrote:
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -156,7 +156,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
>  		"kvm_spurious_fault",
>  		"__reiserfs_panic",
>  		"lbug_with_loc",
> -		"fortify_panic",
> +		"fortify_overflow",
>  	};

If the function is no longer '__noreturn' then this entry needs to be
removed from the global_noreturns list.

-- 
Josh

[toc] | [prev] | [next] | [standalone]


#1697579

FromKees Cook <keescook@chromium.org>
Date2017-07-26 23:40 +0200
Message-ID<u7Cca-5FZ-21@gated-at.bofh.it>
In reply to#1697529
On Wed, Jul 26, 2017 at 12:55 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Jul 26, 2017 at 11:52:19AM -0700, Kees Cook wrote:
>> --- a/tools/objtool/check.c
>> +++ b/tools/objtool/check.c
>> @@ -156,7 +156,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
>>               "kvm_spurious_fault",
>>               "__reiserfs_panic",
>>               "lbug_with_loc",
>> -             "fortify_panic",
>> +             "fortify_overflow",
>>       };
>
> If the function is no longer '__noreturn' then this entry needs to be
> removed from the global_noreturns list.

Ah, dur, yes. Thanks! I see akpm has already merged this correction for -mm.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]


#1699268

Fromkbuild test robot <lkp@intel.com>
Date2017-07-29 11:00 +0200
Message-ID<u8vLj-8c2-13@gated-at.bofh.it>
In reply to#1697485

[Multipart message — attachments visible in raw view] — view raw

Hi Kees,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.13-rc2 next-20170728]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/fortify-Use-WARN-instead-of-BUG-for-now/20170728-091210
config: ia64-sim_defconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All errors (new ones prefixed by >>):

   lib/string.o: In function `fortify_overflow':
>> lib/string.c:984: undefined reference to `warn_slowpath_fmt'

vim +984 lib/string.c

   981	
   982	void fortify_overflow(const char *name)
   983	{
 > 984		WARN(1, "detected buffer overflow in %s\n", name);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web