* [PATCH] check for invalid flags in xfs_attrlist_by_handle
@ 2008-05-31 7:58 Christoph Hellwig
2008-06-20 5:41 ` Timothy Shimmin
0 siblings, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2008-05-31 7:58 UTC (permalink / raw)
To: xfs
xfs_attrlist_by_handle should only take the ATTR_ flags for the root
namespaces. The ATTR_KERN* flags may change at anytime and expect special
preconditions that can't be guaranteed for userspace-originating
requests. For example passing down ATTR_KERNNOVAL through
xfs_attrlist_by_handle will hit an assert in debug builds currently.
Signed-off-by: Christoph Hellwig <hch@lst.de>
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-05-28 17:37:02.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c 2008-05-28 17:42:18.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_TRUST|ATTR_SECURE))
+ return -XFS_ERROR(EINVAL);
+
error = xfs_vget_fsop_handlereq(mp, parinode, &al_hreq.hreq, &inode);
if (error)
goto out;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle
2008-05-31 7:58 [PATCH] check for invalid flags in xfs_attrlist_by_handle Christoph Hellwig
@ 2008-06-20 5:41 ` Timothy Shimmin
2008-06-20 5:48 ` Christoph Hellwig
2008-06-23 11:39 ` Christoph Hellwig
0 siblings, 2 replies; 5+ messages in thread
From: Timothy Shimmin @ 2008-06-20 5:41 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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);
Though would probably then need to include the right header (xfs_attr_leaf.h) for it...
--Tim
Christoph Hellwig wrote:
> xfs_attrlist_by_handle should only take the ATTR_ flags for the root
> namespaces. The ATTR_KERN* flags may change at anytime and expect special
> preconditions that can't be guaranteed for userspace-originating
> requests. For example passing down ATTR_KERNNOVAL through
> xfs_attrlist_by_handle will hit an assert in debug builds currently.
>
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> 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-05-28 17:37:02.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c 2008-05-28 17:42:18.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_TRUST|ATTR_SECURE))
> + return -XFS_ERROR(EINVAL);
> +
> error = xfs_vget_fsop_handlereq(mp, parinode, &al_hreq.hreq, &inode);
> if (error)
> goto out;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle
2008-06-20 5:41 ` Timothy Shimmin
@ 2008-06-20 5:48 ` Christoph Hellwig
2008-06-23 11:39 ` Christoph Hellwig
1 sibling, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2008-06-20 5:48 UTC (permalink / raw)
To: Timothy Shimmin; +Cc: Christoph Hellwig, xfs
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);
>
> Though would probably then need to include the right header (xfs_attr_leaf.h) for it...
Makes sense. I'll revise the patch and send an updated version after
running it through QA.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle
2008-06-20 5:41 ` Timothy Shimmin
2008-06-20 5:48 ` Christoph Hellwig
@ 2008-06-23 11:39 ` Christoph Hellwig
2008-06-24 2:47 ` Timothy Shimmin
1 sibling, 1 reply; 5+ messages in thread
From: Christoph Hellwig @ 2008-06-23 11:39 UTC (permalink / raw)
To: Timothy Shimmin; +Cc: xfs
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.
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:
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;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] check for invalid flags in xfs_attrlist_by_handle
2008-06-23 11:39 ` Christoph Hellwig
@ 2008-06-24 2:47 ` Timothy Shimmin
0 siblings, 0 replies; 5+ messages in thread
From: Timothy Shimmin @ 2008-06-24 2:47 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfs
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;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-06-24 2:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-31 7:58 [PATCH] check for invalid flags in xfs_attrlist_by_handle Christoph Hellwig
2008-06-20 5:41 ` Timothy Shimmin
2008-06-20 5:48 ` Christoph Hellwig
2008-06-23 11:39 ` Christoph Hellwig
2008-06-24 2:47 ` Timothy Shimmin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox