xfs
[Top] [All Lists]

Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle
From: Timothy Shimmin <tes@xxxxxxx>
Date: Tue, 24 Jun 2008 12:47:40 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20080623113946.GA32665@xxxxxx>
References: <20080531075829.GA5424@xxxxxx> <485B431F.2070905@xxxxxxx> <20080623113946.GA32665@xxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.14 (Macintosh/20080421)
Christoph Hellwig wrote:
> On Fri, Jun 20, 2008 at 03:41:51PM +1000, Timothy Shimmin wrote:
>> Fair enough.
>> Actually, I think we only use ATTR_ROOT and ATTR_SECURE for the
>> namespace flags.
>> So you could probably use: XFS_ATTR_NSP_ARGS
>> xfs_attr_leaf.h:#define XFS_ATTR_NSP_ARGS_MASK        (ATTR_ROOT | 
>> ATTR_SECURE)
>> xfs_attr_leaf.h:#define XFS_ATTR_NSP_ARGS(flags) ((flags) & 
>> XFS_ATTR_NSP_ARGS_MASK)
>> and something like:
>>
>> if (!XFS_ATTR_NSP_ARGS(al_hreq.flags))
>>          return -XFS_ERROR(EINVAL);
> 
> Actually a zero flags is of course valid too.
> 
Ah, that would be why I used the phrase "something like" ;-))
Good pt.

> So the check should be & ~(ATTR_ROOT | ATTR_SECURE).  I could use
> XFS_ATTR_NSP_ARGS_MASK but that would pull in not just xfs_attr_leaf.h
> but also xfs_da_btree.h and that needs even more headers..
> 
> So I propose this simple version:
> 
Cool.
I was just thinking about centralising stuff in case we extend the
namespaces. But that's fine.
I'll check it in...

--Tim

> 
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c   2008-06-20 
> 08:17:13.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c        2008-06-23 
> 13:38:17.000000000 +0200
> @@ -470,6 +470,12 @@ xfs_attrlist_by_handle(
>       if (al_hreq.buflen > XATTR_LIST_MAX)
>               return -XFS_ERROR(EINVAL);
>  
> +     /*
> +      * Reject flags, only allow namespaces.
> +      */
> +     if (al_hreq.flags & ~(ATTR_ROOT | ATTR_SECURE))
> +             return -XFS_ERROR(EINVAL);
> +
>       error = xfs_vget_fsop_handlereq(mp, parinode, &al_hreq.hreq, &inode);
>       if (error)
>               goto out;
> 


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