| Message ID | 20171002084119.3504771-1-arnd@arndb.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 2, 2017 at 10:40 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > void fortify_panic(const char *name) __noreturn __cold; > + > +/* work around GCC PR82365 */ > +#if defined(CONFIG_KASAN) && !defined(__clang__) && GCC_VERSION <= 80000 > +#define fortify_panic(x) \ > + do { \ > + asm volatile(""); \ > + fortify_panic(x); \ > + } while (0) > +#endif This broke the build for the fortify_panic() definition in lib/string.c which clashes with the macro. I've fixed it locally by renaming it to __fortify_panic, but won't post the fixed version until I get some feedback on the basic approach. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/02/2017 11:40 AM, Arnd Bergmann wrote: > The hardened strlen() function causes rather large stack usage > in at least one file in the kernel when CONFIG_KASAN is enabled: > > drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init': > drivers/media/usb/em28xx/em28xx-dvb.c:2062:1: error: the frame size of 3256 bytes is larger than 204 bytes [-Werror=frame-larger-than=] > > Analyzing this problem led to the discovery that gcc fails to > merge the stack slots for the i2c_board_info[] structures after > we strlcpy() into them, due to the 'noreturn' attribute on the > source string length check. > > The compiler behavior should get fixed in gcc-8, but for users > of existing gcc versions, we can work around it using an empty > inline assembly statement before the call to fortify_panic(). > > The workaround is unfortunately very ugly, and I tried my best > to limit it being applied to affected versions of gcc when > KASAN is used. Alternative suggestions welcome. > I don't have a really strong preference, so this approach is fine by me, but s/strlcpy/[strncpy|memcpy] approach seems a little better to me, because it's not ugly. This ugly workaround would make more sense if we a had lot of cases like in em28xx_dvb_init(). > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365 > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/linux/string.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/linux/string.h b/include/linux/string.h > index c7a1132cdc93..1bf5ecdf8e01 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -228,6 +228,16 @@ static inline const char *kbasename(const char *path) > #define __RENAME(x) __asm__(#x) > > void fortify_panic(const char *name) __noreturn __cold; > + > +/* work around GCC PR82365 */ > +#if defined(CONFIG_KASAN) && !defined(__clang__) && GCC_VERSION <= 80000 > +#define fortify_panic(x) \ > + do { \ > + asm volatile(""); \ > + fortify_panic(x); \ > + } while (0) > +#endif > + > 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 __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter"); > -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arnd,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.14-rc3 next-20170929]
[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/Arnd-Bergmann/string-h-work-around-for-increased-stack-usage/20171003-210611
config: x86_64-randconfig-ws0-10040032 (attached as .config)
compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
In file included from lib/string.c:23:0:
>> include/linux/string.h:235:2: error: expected identifier or '(' before 'do'
do { \
^
>> lib/string.c:1048:6: note: in expansion of macro 'fortify_panic'
void fortify_panic(const char *name)
^
>> include/linux/string.h:238:4: error: expected identifier or '(' before 'while'
} while (0)
^
>> lib/string.c:1048:6: note: in expansion of macro 'fortify_panic'
void fortify_panic(const char *name)
^
vim +235 include/linux/string.h
231
232 /* work around GCC PR82365 */
233 #if defined(CONFIG_KASAN) && !defined(__clang__) && GCC_VERSION <= 80000
234 #define fortify_panic(x) \
> 235 do { \
236 asm volatile(""); \
237 fortify_panic(x); \
> 238 } while (0)
239 #endif
240
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/include/linux/string.h b/include/linux/string.h index c7a1132cdc93..1bf5ecdf8e01 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -228,6 +228,16 @@ static inline const char *kbasename(const char *path) #define __RENAME(x) __asm__(#x) void fortify_panic(const char *name) __noreturn __cold; + +/* work around GCC PR82365 */ +#if defined(CONFIG_KASAN) && !defined(__clang__) && GCC_VERSION <= 80000 +#define fortify_panic(x) \ + do { \ + asm volatile(""); \ + fortify_panic(x); \ + } while (0) +#endif + 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 __read_overflow3(void) __compiletime_error("detected read beyond size of object passed as 3rd parameter");
The hardened strlen() function causes rather large stack usage in at least one file in the kernel when CONFIG_KASAN is enabled: drivers/media/usb/em28xx/em28xx-dvb.c: In function 'em28xx_dvb_init': drivers/media/usb/em28xx/em28xx-dvb.c:2062:1: error: the frame size of 3256 bytes is larger than 204 bytes [-Werror=frame-larger-than=] Analyzing this problem led to the discovery that gcc fails to merge the stack slots for the i2c_board_info[] structures after we strlcpy() into them, due to the 'noreturn' attribute on the source string length check. The compiler behavior should get fixed in gcc-8, but for users of existing gcc versions, we can work around it using an empty inline assembly statement before the call to fortify_panic(). The workaround is unfortunately very ugly, and I tried my best to limit it being applied to affected versions of gcc when KASAN is used. Alternative suggestions welcome. Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365 Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- include/linux/string.h | 10 ++++++++++ 1 file changed, 10 insertions(+)