Date: Thu, 25 Jun 2015 08:09:47 +0200
From: Willy Tarreau
To: Linus Torvalds
Cc: Andy Lutomirski, "security at kernel.org", halfdog, Kees Cook, Al Viro
Subject: Re: Group man to group root privilege escalation
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Jun 24, 2015 at 03:06:21PM -0700, Linus Torvalds wrote:
> On Wed, Jun 24, 2015 at 2:45 PM, Linus Torvalds wrote:
> >
> > Ahh, yes. Ok, I agree, we should probably do both - use f_cred for the
> > suid removal check, *and* make sure that group-sticky directories
> > clear the sgid bit on file creation.
> 
> Maybe something fairly simple like this for the SGID case?
> 
>                     Linus

>  fs/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index e8d62688ed91..7bde0ad673e4 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1898,6 +1898,8 @@ void inode_init_owner(struct inode *inode, const struct inode *dir,
>  		inode->i_gid = dir->i_gid;
>  		if (S_ISDIR(mode))
>  			mode |= S_ISGID;
> +		else if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
> +			mode &= ~S_ISGID;

Why limit it only to cases where S_IXGRP is set ? Wouldn't it be more
consistent to always clear it ? That's easier to document and remember
in my opinion, that would basically just be :

+		else
+			mode &= ~S_ISGID;

Am I missing something ?

Willy