linux-kernel - Re: [PATCH] string.h: work around for increased stack usage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 2 Oct 2017 17:07:42 +0300
From:   Andrey Ryabinin <aryabinin@...tuozzo.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     David Laight <David.Laight@...lab.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        "David S . Miller" <davem@...emloft.net>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-media @ vger . kernel . org" <linux-media@...r.kernel.org>,
        "linux-kernel @ vger . kernel . org" <linux-kernel@...r.kernel.org>,
        "kasan-dev @ googlegroups . com" <kasan-dev@...glegroups.com>,
        "linux-kbuild @ vger . kernel . org" <linux-kbuild@...r.kernel.org>
Subject: Re: [PATCH] string.h: work around for increased stack usage

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@...db.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");
>