linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fortify: Avoid panic() in favor of BUG()
@ 2017-06-26 23:51 Kees Cook
  2017-06-27 15:52 ` kbuild test robot
  0 siblings, 1 reply; 3+ messages in thread
From: Kees Cook @ 2017-06-26 23:51 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jonathan Corbet, Mika Westerberg, Bjorn Helgaas,
	Mauro Carvalho Chehab, Heikki Krogerus, Daniel Micay,
	linux-kernel

Since detection of a given fortify failure is sufficient to stop the
memory corruption from happening, it doesn't make sense to unconditionally
bring down the entire system. Instead, use BUG() which will stop the bad
thread of kernel execution (and only optionally panic the system).

Cc: Daniel Micay <danielmicay@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 lib/string.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index a6ee1955a701..ebbb99c775bd 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -981,6 +981,7 @@ EXPORT_SYMBOL(strreplace);
 
 void fortify_panic(const char *name)
 {
-	panic("detected buffer overflow in %s", name);
+	pr_emerg("detected buffer overflow in %s\n", name);
+	BUG();
 }
 EXPORT_SYMBOL(fortify_panic);
-- 
2.7.4


-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] fortify: Avoid panic() in favor of BUG()
  2017-06-26 23:51 [PATCH] fortify: Avoid panic() in favor of BUG() Kees Cook
@ 2017-06-27 15:52 ` kbuild test robot
  2017-06-27 19:16   ` Kees Cook
  0 siblings, 1 reply; 3+ messages in thread
From: kbuild test robot @ 2017-06-27 15:52 UTC (permalink / raw)
  To: Kees Cook
  Cc: kbuild-all, Andrew Morton, Jonathan Corbet, Mika Westerberg,
	Bjorn Helgaas, Mauro Carvalho Chehab, Heikki Krogerus,
	Daniel Micay, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

Hi Kees,

[auto build test WARNING on next-20170619]
[cannot apply to linus/master linux/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc7]
[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-Avoid-panic-in-favor-of-BUG/20170627-230653
config: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
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=sh 

All warnings (new ones prefixed by >>):

   lib/string.c: In function 'fortify_panic':
>> lib/string.c:986:1: warning: 'noreturn' function does return
    }
    ^

vim +/noreturn +986 lib/string.c

94df2904 Rasmus Villemoes 2015-06-25  970   *
94df2904 Rasmus Villemoes 2015-06-25  971   * Returns pointer to the nul byte at the end of @s.
94df2904 Rasmus Villemoes 2015-06-25  972   */
94df2904 Rasmus Villemoes 2015-06-25  973  char *strreplace(char *s, char old, char new)
94df2904 Rasmus Villemoes 2015-06-25  974  {
94df2904 Rasmus Villemoes 2015-06-25  975  	for (; *s; ++s)
94df2904 Rasmus Villemoes 2015-06-25  976  		if (*s == old)
94df2904 Rasmus Villemoes 2015-06-25  977  			*s = new;
94df2904 Rasmus Villemoes 2015-06-25  978  	return s;
94df2904 Rasmus Villemoes 2015-06-25  979  }
94df2904 Rasmus Villemoes 2015-06-25  980  EXPORT_SYMBOL(strreplace);
b90d6eba Daniel Micay     2017-06-06  981  
b90d6eba Daniel Micay     2017-06-06  982  void fortify_panic(const char *name)
b90d6eba Daniel Micay     2017-06-06  983  {
8a94b337 Kees Cook        2017-06-26  984  	pr_emerg("detected buffer overflow in %s\n", name);
8a94b337 Kees Cook        2017-06-26  985  	BUG();
b90d6eba Daniel Micay     2017-06-06 @986  }
b90d6eba Daniel Micay     2017-06-06  987  EXPORT_SYMBOL(fortify_panic);

:::::: The code at line 986 was first introduced by commit
:::::: b90d6eba50d76750576dc8afe5eaa43974a1f17f include/linux/string.h: add the option of fortified string.h functions

:::::: TO: Daniel Micay <danielmicay@gmail.com>
:::::: CC: Kees Cook <keescook@chromium.org>

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45863 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] fortify: Avoid panic() in favor of BUG()
  2017-06-27 15:52 ` kbuild test robot
@ 2017-06-27 19:16   ` Kees Cook
  0 siblings, 0 replies; 3+ messages in thread
From: Kees Cook @ 2017-06-27 19:16 UTC (permalink / raw)
  To: kbuild test robot, Yoshinori Sato, Rich Felker, linux-sh
  Cc: kbuild-all, Andrew Morton, Jonathan Corbet, Mika Westerberg,
	Bjorn Helgaas, Mauro Carvalho Chehab, Heikki Krogerus,
	Daniel Micay, LKML

On Tue, Jun 27, 2017 at 8:52 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Kees,
>
> [auto build test WARNING on next-20170619]
> [cannot apply to linus/master linux/master v4.12-rc6 v4.12-rc5 v4.12-rc4 v4.12-rc7]
> [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-Avoid-panic-in-favor-of-BUG/20170627-230653
> config: sh-allyesconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> 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=sh
>
> All warnings (new ones prefixed by >>):
>
>    lib/string.c: In function 'fortify_panic':
>>> lib/string.c:986:1: warning: 'noreturn' function does return
>     }
>     ^
>
> vim +/noreturn +986 lib/string.c
>
> 94df2904 Rasmus Villemoes 2015-06-25  970   *
> 94df2904 Rasmus Villemoes 2015-06-25  971   * Returns pointer to the nul byte at the end of @s.
> 94df2904 Rasmus Villemoes 2015-06-25  972   */
> 94df2904 Rasmus Villemoes 2015-06-25  973  char *strreplace(char *s, char old, char new)
> 94df2904 Rasmus Villemoes 2015-06-25  974  {
> 94df2904 Rasmus Villemoes 2015-06-25  975       for (; *s; ++s)
> 94df2904 Rasmus Villemoes 2015-06-25  976               if (*s == old)
> 94df2904 Rasmus Villemoes 2015-06-25  977                       *s = new;
> 94df2904 Rasmus Villemoes 2015-06-25  978       return s;
> 94df2904 Rasmus Villemoes 2015-06-25  979  }
> 94df2904 Rasmus Villemoes 2015-06-25  980  EXPORT_SYMBOL(strreplace);
> b90d6eba Daniel Micay     2017-06-06  981
> b90d6eba Daniel Micay     2017-06-06  982  void fortify_panic(const char *name)
> b90d6eba Daniel Micay     2017-06-06  983  {
> 8a94b337 Kees Cook        2017-06-26  984       pr_emerg("detected buffer overflow in %s\n", name);
> 8a94b337 Kees Cook        2017-06-26  985       BUG();
> b90d6eba Daniel Micay     2017-06-06 @986  }
> b90d6eba Daniel Micay     2017-06-06  987  EXPORT_SYMBOL(fortify_panic);

This implies that the sh arch doesn't correctly mark their BUG
implementation as not returning... I'll send a patch.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-27 19:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-26 23:51 [PATCH] fortify: Avoid panic() in favor of BUG() Kees Cook
2017-06-27 15:52 ` kbuild test robot
2017-06-27 19:16   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).