xfs
[Top] [All Lists]

Re: [PATCH 1/2] kill attr_capable callbacks

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [PATCH 1/2] kill attr_capable callbacks
From: Timothy Shimmin <tes@xxxxxxx>
Date: Mon, 12 May 2008 16:22:37 +1000
Cc: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20080512014721.GV155679365@xxxxxxx>
References: <20080430112213.GA16966@xxxxxx> <20080512014721.GV155679365@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.14 (Macintosh/20080421)
David Chinner wrote:
> Tim,
> 
> Seeing you are working on xattr stuff right now, can you pick this up?
> (and the followup patch as well?)
> 
> Cheers,
> 
> Dave.
> 
Yep.

--Tim

> On Wed, Apr 30, 2008 at 01:22:13PM +0200, Christoph Hellwig wrote:
>> No need for addition permission checks in the xattr handler,
>> fs/xattr.c:xattr_permission() already does them, and in fact slightly
>> more strict then what was in the attr_capable handlers.
>>
>>
>> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>>
>> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c
>> ===================================================================
>> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_iops.c   2008-04-29 
>> 21:32:56.000000000 +0200
>> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_iops.c        2008-04-29 
>> 21:33:30.000000000 +0200
>> @@ -747,15 +747,11 @@ xfs_vn_setxattr(
>>      char            *attr = (char *)name;
>>      attrnames_t     *namesp;
>>      int             xflags = 0;
>> -    int             error;
>>  
>>      namesp = attr_lookup_namespace(attr, attr_namespaces, ATTR_NAMECOUNT);
>>      if (!namesp)
>>              return -EOPNOTSUPP;
>>      attr += namesp->attr_namelen;
>> -    error = namesp->attr_capable(vp, NULL);
>> -    if (error)
>> -            return error;
>>  
>>      /* Convert Linux syscall to XFS internal ATTR flags */
>>      if (flags & XATTR_CREATE)
>> @@ -777,15 +773,11 @@ xfs_vn_getxattr(
>>      char            *attr = (char *)name;
>>      attrnames_t     *namesp;
>>      int             xflags = 0;
>> -    ssize_t         error;
>>  
>>      namesp = attr_lookup_namespace(attr, attr_namespaces, ATTR_NAMECOUNT);
>>      if (!namesp)
>>              return -EOPNOTSUPP;
>>      attr += namesp->attr_namelen;
>> -    error = namesp->attr_capable(vp, NULL);
>> -    if (error)
>> -            return error;
>>  
>>      /* Convert Linux syscall to XFS internal ATTR flags */
>>      if (!size) {
>> @@ -825,15 +817,12 @@ xfs_vn_removexattr(
>>      char            *attr = (char *)name;
>>      attrnames_t     *namesp;
>>      int             xflags = 0;
>> -    int             error;
>>  
>>      namesp = attr_lookup_namespace(attr, attr_namespaces, ATTR_NAMECOUNT);
>>      if (!namesp)
>>              return -EOPNOTSUPP;
>>      attr += namesp->attr_namelen;
>> -    error = namesp->attr_capable(vp, NULL);
>> -    if (error)
>> -            return error;
>> +
>>      xflags |= namesp->attr_flag;
>>      return namesp->attr_remove(vp, attr, xflags);
>>  }
>> Index: linux-2.6-xfs/fs/xfs/xfs_attr.c
>> ===================================================================
>> --- linux-2.6-xfs.orig/fs/xfs/xfs_attr.c     2008-04-29 21:32:25.000000000 
>> +0200
>> +++ linux-2.6-xfs/fs/xfs/xfs_attr.c  2008-04-29 21:32:51.000000000 +0200
>> @@ -2622,43 +2622,6 @@ attr_lookup_namespace(
>>      return NULL;
>>  }
>>  
>> -/*
>> - * Some checks to prevent people abusing EAs to get over quota:
>> - * - Don't allow modifying user EAs on devices/symlinks;
>> - * - Don't allow modifying user EAs if sticky bit set;
>> - */
>> -STATIC int
>> -attr_user_capable(
>> -    bhv_vnode_t     *vp,
>> -    cred_t          *cred)
>> -{
>> -    struct inode    *inode = vn_to_inode(vp);
>> -
>> -    if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
>> -            return -EPERM;
>> -    if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode) &&
>> -        !capable(CAP_SYS_ADMIN))
>> -            return -EPERM;
>> -    if (S_ISDIR(inode->i_mode) && (inode->i_mode & S_ISVTX) &&
>> -        (current_fsuid(cred) != inode->i_uid) && !capable(CAP_FOWNER))
>> -            return -EPERM;
>> -    return 0;
>> -}
>> -
>> -STATIC int
>> -attr_trusted_capable(
>> -    bhv_vnode_t     *vp,
>> -    cred_t          *cred)
>> -{
>> -    struct inode    *inode = vn_to_inode(vp);
>> -
>> -    if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
>> -            return -EPERM;
>> -    if (!capable(CAP_SYS_ADMIN))
>> -            return -EPERM;
>> -    return 0;
>> -}
>> -
>>  STATIC int
>>  attr_system_set(
>>      bhv_vnode_t *vp, char *name, void *data, size_t size, int xflags)
>> @@ -2709,7 +2672,6 @@ struct attrnames attr_system = {
>>      .attr_get       = attr_system_get,
>>      .attr_set       = attr_system_set,
>>      .attr_remove    = attr_system_remove,
>> -    .attr_capable   = (attrcapable_t)fs_noerr,
>>  };
>>  
>>  struct attrnames attr_trusted = {
>> @@ -2719,7 +2681,6 @@ struct attrnames attr_trusted = {
>>      .attr_get       = attr_generic_get,
>>      .attr_set       = attr_generic_set,
>>      .attr_remove    = attr_generic_remove,
>> -    .attr_capable   = attr_trusted_capable,
>>  };
>>  
>>  struct attrnames attr_secure = {
>> @@ -2729,7 +2690,6 @@ struct attrnames attr_secure = {
>>      .attr_get       = attr_generic_get,
>>      .attr_set       = attr_generic_set,
>>      .attr_remove    = attr_generic_remove,
>> -    .attr_capable   = (attrcapable_t)fs_noerr,
>>  };
>>  
>>  struct attrnames attr_user = {
>> @@ -2738,7 +2698,6 @@ struct attrnames attr_user = {
>>      .attr_get       = attr_generic_get,
>>      .attr_set       = attr_generic_set,
>>      .attr_remove    = attr_generic_remove,
>> -    .attr_capable   = attr_user_capable,
>>  };
>>  
>>  struct attrnames *attr_namespaces[] =
>> Index: linux-2.6-xfs/fs/xfs/xfs_attr.h
>> ===================================================================
>> --- linux-2.6-xfs.orig/fs/xfs/xfs_attr.h     2008-04-29 21:33:38.000000000 
>> +0200
>> +++ linux-2.6-xfs/fs/xfs/xfs_attr.h  2008-04-29 21:33:52.000000000 +0200
>> @@ -42,7 +42,6 @@ typedef int (*attrset_t)(bhv_vnode_t *, 
>>  typedef int (*attrget_t)(bhv_vnode_t *, char *, void *, size_t, int);
>>  typedef int (*attrremove_t)(bhv_vnode_t *, char *, int);
>>  typedef int (*attrexists_t)(bhv_vnode_t *);
>> -typedef int (*attrcapable_t)(bhv_vnode_t *, struct cred *);
>>  
>>  typedef struct attrnames {
>>      char *          attr_name;
>> @@ -52,7 +51,6 @@ typedef struct attrnames {
>>      attrset_t       attr_set;
>>      attrremove_t    attr_remove;
>>      attrexists_t    attr_exists;
>> -    attrcapable_t   attr_capable;
>>  } attrnames_t;
>>  
>>  #define ATTR_NAMECOUNT      4
>>
> 


<Prev in Thread] Current Thread [Next in Thread>