linux-kernel - Re: [PATCH] cred/userns: define current_user

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] [day] [month] [year] [list]

Date:	Tue, 15 Mar 2016 11:34:36 -0500
From:	"Serge E. Hallyn" <serge@...lyn.com>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	Serge Hallyn <serge.hallyn@...onical.com>,
	David Howells <dhowells@...hat.com>,
	Yaowei Bai <baiyaowei@...s.chinamobile.com>,
	James Morris <james.l.morris@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cred/userns: define current_user_ns() as a function

On Mon, Mar 14, 2016 at 10:47:33PM +0100, Arnd Bergmann wrote:
> The current_user_ns() macro currently returns &init_user_ns when
> user namespaces are disabled, and that causes several warnings
> when building with gcc-6.0 in code that compares the result of
> the macro to &init_user_ns itself:
> 
> fs/xfs/xfs_ioctl.c: In function 'xfs_ioctl_setattr_check_projid':
> fs/xfs/xfs_ioctl.c:1249:22: error: self-comparison always evaluates to true [-Werror=tautological-compare]
>   if (current_user_ns() == &init_user_ns)
> 
> This is a legitimate warning in principle, but here it isn't
> really helpful, so I'm reprasing the definition in a way that
> shuts up the warning. Apparently gcc only warns when comparing
> identical literals, but it can figure out that the result of
> an inline function can be identical to a constant expression
> in order to optimize a condition yet not warn about the fact
> that the condition is known at compile time. This is exactly
> what we want here, and it looks reasonable because we generally
> prefer inline functions over macros anyway.
> 
> Signed-off-by: Arnd Bergmann <arnd@...db.de>

Was a bit worried about the first hunk since capability.h doesn't
include cred.h explicitly, but I guess if that was a problem it
would've not compiled long ago due to current_cred_xxx().

Acked-by: Serge Hallyn <serge.hallyn@...onical.com>

> ---
>  include/linux/capability.h | 2 --
>  include/linux/cred.h       | 5 ++++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index f314275d4e3f..00690ff92edf 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -40,8 +40,6 @@ struct inode;
>  struct dentry;
>  struct user_namespace;
>  
> -struct user_namespace *current_user_ns(void);
> -
>  extern const kernel_cap_t __cap_empty_set;
>  extern const kernel_cap_t __cap_init_eff_set;
>  
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 8d70e1361ecd..257db64562e5 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -377,7 +377,10 @@ extern struct user_namespace init_user_ns;
>  #ifdef CONFIG_USER_NS
>  #define current_user_ns()	(current_cred_xxx(user_ns))
>  #else
> -#define current_user_ns()	(&init_user_ns)
> +static inline struct user_namespace *current_user_ns(void)
> +{
> +	return &init_user_ns;
> +}
>  #endif
>  
>  
> -- 
> 2.7.0