netdev - Re: [PATCH 24/26] ocfs2: reduce stack size with KASAN

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]

Message-ID: <1488476770.2179.6.camel@perches.com>
Date:   Thu, 02 Mar 2017 09:46:10 -0800
From:   Joe Perches <joe@...ches.com>
To:     Arnd Bergmann <arnd@...db.de>, kasan-dev@...glegroups.com
Cc:     Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Alexander Potapenko <glider@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
        linux-wireless@...r.kernel.org,
        kernel-build-reports@...ts.linaro.org,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH 24/26] ocfs2: reduce stack size with KASAN

On Thu, 2017-03-02 at 17:38 +0100, Arnd Bergmann wrote:
> The internal logging infrastructure in ocfs2 causes special warning code to be
> used with KASAN, which produces rather large stack frames:

> fs/ocfs2/super.c: In function 'ocfs2_fill_super':
> fs/ocfs2/super.c:1219:1: error: the frame size of 3264 bytes is larger than 3072 bytes [-Werror=frame-larger-than=]

At least by default it doesn't seem to.

gcc 6.2 allyesconfig, CONFIG_KASAN=y
with either CONFIG_KASAN_INLINE or CONFIG_KASAN_OUTLINE

gcc doesn't emit a stack warning

> By simply passing the mask by value instead of reference, we can avoid the
> problem completely.

Any idea why that's so?
 
>  On 64-bit architectures, this is also more efficient,

Efficient true, but the same overall stack no?

> while on the less common (at least among ocfs2 users) 32-bit architectures,
> I'm guessing that the resulting code is comparable to what it was before.
> 
> The current version was introduced by Joe Perches as an optimization, maybe
> he can see if my change regresses compared to his.

I don't see it.

> Cc: Joe Perches <joe@...ches.com>
> Fixes: 7c2bd2f930ae ("ocfs2: reduce object size of mlog uses")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  fs/ocfs2/cluster/masklog.c | 10 +++++-----
>  fs/o cfs2/cluster/masklog.h |  4 ++--
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ocfs2/cluster/masklog.c b/fs/ocfs2/cluster/masklog.c
> index d331c2386b94..9720c5443e4d 100644
> --- a/fs/ocfs2/cluster/masklog.c
> +++ b/fs/ocfs2/cluster/masklog.c
> @@ -64,7 +64,7 @@ static ssize_t mlog_mask_store(u64 mask, const char *buf, size_t count)
>  	return count;
>  }
>  
> -void __mlog_printk(const u64 *mask, const char *func, int line,
> +void __mlog_printk(const u64 mask, const char *func, int line,
>  		   const char *fmt, ...)
>  {
>  	struct va_format vaf;
> @@ -72,14 +72,14 @@ void __mlog_printk(const u64 *mask, const char *func, int line,
>  	const char *level;
>  	const char *prefix = "";
>  
> -	if (!__mlog_test_u64(*mask, mlog_and_bits) ||
> -	    __mlog_test_u64(*mask, mlog_not_bits))
> +	if (!__mlog_test_u64(mask, mlog_and_bits) ||
> +	    __mlog_test_u64(mask, mlog_not_bits))
>  		return;
>  
> -	if (*mask & ML_ERROR) {
> +	if (mask & ML_ERROR) {
>  		level = KERN_ERR;
>  		prefix = "ERROR: ";
> -	} else if (*mask & ML_NOTICE) {
> +	} else if (mask & ML_NOTICE) {
>  		level = KERN_NOTICE;
>  	} else {
>  		level = KERN_INFO;
> diff --git a/fs/ocfs2/cluster/masklog.h b/fs/ocfs2/cluster/masklog.h
> index 308ea0eb35fd..0d0f4bf2c3d8 100644
> --- a/fs/ocfs2/cluster/masklog.h
> +++ b/fs/ocfs2/cluster/masklog.h
> @@ -163,7 +163,7 @@ extern struct mlog_bits mlog_and_bits, mlog_not_bits;
>  #endif
>  
>  __printf(4, 5)
> -void __mlog_printk(const u64 *m, const char *func, int line,
> +void __mlog_printk(const u64 m, const char *func, int line,
>  		   const char *fmt, ...);
>  
>  /*
> @@ -174,7 +174,7 @@ void __mlog_printk(const u64 *m, const char *func, int line,
>  do {									\
>  	u64 _m = MLOG_MASK_PREFIX | (mask);				\
>  	if (_m & ML_ALLOWED_BITS)					\
> -		__mlog_printk(&_m, __func__, __LINE__, fmt,		\
> +		__mlog_printk(_m, __func__, __LINE__, fmt,		\
>  			      ##__VA_ARGS__);				\
>  } while (0)
>