Re: remove_suid bangs on xattrs

[Chris Mason <chris.mason@oracle.com>]

[ 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
>