fortify: Avoid panic() in favor of BUG()

PATCH In review Kees Cook 2 participants 3 messages Jun 26, 2017 – Jun 27, 2017

Subsystems (3)

Library code SuperH Generic string library

lore

Share & tools

lore

Kees Cook

author

2017-06-26 23:51 UTC #0 lore raw

PATCH 1/1 fortify: Avoid panic() in favor of BUG()

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 <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
 lib/string.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

lib/string.c +2 -1

index a6ee1955a701..ebbb99c775bd 100644
@@ -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);

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

kbuild test robot

build bot

2017-06-27 15:53 UTC #1 lore raw

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 <[email protected]>
:::::: CC: Kees Cook <[email protected]>

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

Kees Cook

author

2017-06-27 19:16 UTC #2 lore raw

Quoted 34 lines · kbuild test robot

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':
Quoted 1 line
Quoted 1 line
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  }

Quoted 8 lines

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

Signature · 2 lines

Kees Cook
Pixel Security