On Fri, Jul 27, 2007 at 12:31:11PM -0500, Serge E. Hallyn wrote:
> When you
> 
>       setfcaps -c cap_net_admin=p -e /bin/ping
>       cp /bin/sh /bin/ping
> 
> then /bin/ping should lose its file capabilities.  This patch probably
> will need to be cleaned up, but seems to work as it should.

Example is bad -- cp will unlink /bin/ping and put a new inode in it's
place. dd would convey what happens better :)

>  }
>  EXPORT_SYMBOL(inode_setattr);
>  
> +int file_has_capabilities(struct dentry *dentry);
> +int remove_file_capabilities(struct dentry *dentry);
> +
>  int notify_change(struct dentry * dentry, struct iattr * attr)
>  {
>       struct inode *inode = dentry->d_inode;
> @@ -116,6 +119,15 @@ int notify_change(struct dentry * dentry, struct iattr * 
> attr)
>               attr->ia_atime = now;
>       if (!(ia_valid & ATTR_MTIME_SET))
>               attr->ia_mtime = now;
> +     if (ia_valid & ATTR_KILL_CAP) {
> +             attr->ia_valid &= ~ATTR_KILL_CAP;
> +             ia_valid &= ~ATTR_KILL_CAP;
> +             if (file_has_capabilities(dentry)) {
> +                     error = remove_file_capabilities(dentry);
> +                     if (error)
> +                             return error;
> +             }
> +     }

If the caps can't be removed for whatever reason, then any suid bits
stick around. Is this something to actually worry about?

> +int remove_file_capabilities(struct dentry *dentry);
>  int remove_suid(struct dentry *dentry)
>  {
>       int kill = should_remove_suid(dentry);
> +     int ret = 0;
>  
>       if (unlikely(kill))
> -             return __remove_suid(dentry, kill);
> +             ret = __remove_suid(dentry, kill);
> +     if (ret)
> +             return ret;
>  
> -     return 0;
> +     if (should_remove_cap(dentry))
> +             ret = remove_file_capabilities(dentry);
> +
> +     return ret;
>  }

Again, I'm worried about removing caps but not suid bits. Should I be
worried?

Thanks

Attachment: pgpzkrqo3RxAT.pgp
Description: PGP signature

Reply via email to