From mboxrd@z Thu Jan  1 00:00:00 1970
From: Chris Mason <chris.mason@oracle.com>
Subject: Re: remove_suid bangs on xattrs
Date: Mon, 16 Aug 2010 15:44:39 -0400
Message-ID: <20100816194439.GG993@think>
References: <20100816193812.GF993@think>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
To: linux-fsdevel@vger.kernel.org, serge.hallyn@canonical.com
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from rcsinet10.oracle.com ([148.87.113.121]:50535 "EHLO
	rcsinet10.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756372Ab0HPTrM (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 16 Aug 2010 15:47:12 -0400
Content-Disposition: inline
In-Reply-To: <20100816193812.GF993@think>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

[ sorry, corrected cc list ]

On Mon, Aug 16, 2010 at 03:38:12PM -0400, Chris Mason wrote:
> Hi everyone,
> 
> I'm looking into a 2.6.35 btrfs performance regression, and perf tells
> me that I'm spending a lot of time hammering on xattrs inside
> remove_suid.  This is pretty surprising because I'm running as root, and
> my files are not suid.  Looking back to this commit:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=b53767719b6cd8789392ea3e7e2eb7b8906898f0
> 
> We've changed remove_suid's semantics from
> 
> if (file_is_suid)
>     try to remove it
> 
> To something that always checks to see if we have removal permissions.
> 
> Was this intentional?  It didn't cause my 2.6.35 regression (that's all
> my fault) but it does look wrong to me:
> 
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 4fb1546..79f24a9 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1627,12 +1627,18 @@ int __remove_suid(struct dentry *dentry, int kill)
>  
>  int remove_suid(struct dentry *dentry)
>  {
> -       int kill = should_remove_suid(dentry);
> +       int killsuid = should_remove_suid(dentry);
> +       int killpriv = security_inode_need_killpriv(dentry);
> +       int error = 0;
>  
> -       if (unlikely(kill))
> -               return __remove_suid(dentry, kill);
> +       if (killpriv < 0)
> +               return killpriv;
> +       if (killpriv)
> +               error = security_inode_killpriv(dentry);
> +       if (!error && killsuid)
> +               error = __remove_suid(dentry, killsuid);
>  
> -       return 0;
> +       return error;
>  }
>  EXPORT_SYMBOL(remove_suid);
> 
> -chris
>