All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl
@ 2013-08-01 15:30 Dwight Engen
  2013-08-01 16:27 ` Brian Foster
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Dwight Engen @ 2013-08-01 15:30 UTC (permalink / raw)
  To: xfs

Check for CAP_SYS_ADMIN since the caller can truncate preallocated
blocks from files they do not own nor have write access to. A more
fine grained access check was considered: require the caller to
specify their own uid/gid and to use inode_permission to check for
write, but this would not catch the case of an inode not reachable
via path traversal from the callers mount namespace.

Add check for read-only filesystem to free eofblocks ioctl.

Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
---
 fs/xfs/xfs_ioctl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 60d9d1e..c8f3511 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1620,6 +1620,12 @@ xfs_file_ioctl(
 		struct xfs_fs_eofblocks eofb;
 		struct xfs_eofblocks keofb;
 
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+		if (mp->m_flags & XFS_MOUNT_RDONLY)
+			return -XFS_ERROR(EROFS);
+
 		if (copy_from_user(&eofb, arg, sizeof(eofb)))
 			return -XFS_ERROR(EFAULT);
 
-- 
1.8.1.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl
  2013-08-01 15:30 [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl Dwight Engen
@ 2013-08-01 16:27 ` Brian Foster
  2013-08-02  1:47 ` Dave Chinner
  2013-08-02  2:47 ` Gao feng
  2 siblings, 0 replies; 5+ messages in thread
From: Brian Foster @ 2013-08-01 16:27 UTC (permalink / raw)
  To: Dwight Engen; +Cc: xfs

On 08/01/2013 11:30 AM, Dwight Engen wrote:
> Check for CAP_SYS_ADMIN since the caller can truncate preallocated
> blocks from files they do not own nor have write access to. A more
> fine grained access check was considered: require the caller to
> specify their own uid/gid and to use inode_permission to check for
> write, but this would not catch the case of an inode not reachable
> via path traversal from the callers mount namespace.
> 
> Add check for read-only filesystem to free eofblocks ioctl.
> 
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> ---

Thanks Dwight!

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_ioctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 60d9d1e..c8f3511 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1620,6 +1620,12 @@ xfs_file_ioctl(
>  		struct xfs_fs_eofblocks eofb;
>  		struct xfs_eofblocks keofb;
>  
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +		if (mp->m_flags & XFS_MOUNT_RDONLY)
> +			return -XFS_ERROR(EROFS);
> +
>  		if (copy_from_user(&eofb, arg, sizeof(eofb)))
>  			return -XFS_ERROR(EFAULT);
>  
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl
  2013-08-01 15:30 [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl Dwight Engen
  2013-08-01 16:27 ` Brian Foster
@ 2013-08-02  1:47 ` Dave Chinner
  2013-08-02  2:47 ` Gao feng
  2 siblings, 0 replies; 5+ messages in thread
From: Dave Chinner @ 2013-08-02  1:47 UTC (permalink / raw)
  To: Dwight Engen; +Cc: xfs

On Thu, Aug 01, 2013 at 11:30:20AM -0400, Dwight Engen wrote:
> Check for CAP_SYS_ADMIN since the caller can truncate preallocated
> blocks from files they do not own nor have write access to. A more
> fine grained access check was considered: require the caller to
> specify their own uid/gid and to use inode_permission to check for
> write, but this would not catch the case of an inode not reachable
> via path traversal from the callers mount namespace.
> 
> Add check for read-only filesystem to free eofblocks ioctl.
> 
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>

looks good.

Reviewed-by: Dave Chinner <dchinner@redhat.com>

-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl
  2013-08-01 15:30 [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl Dwight Engen
  2013-08-01 16:27 ` Brian Foster
  2013-08-02  1:47 ` Dave Chinner
@ 2013-08-02  2:47 ` Gao feng
  2 siblings, 0 replies; 5+ messages in thread
From: Gao feng @ 2013-08-02  2:47 UTC (permalink / raw)
  To: Dwight Engen; +Cc: xfs

On 08/01/2013 11:30 PM, Dwight Engen wrote:
> Check for CAP_SYS_ADMIN since the caller can truncate preallocated
> blocks from files they do not own nor have write access to. A more
> fine grained access check was considered: require the caller to
> specify their own uid/gid and to use inode_permission to check for
> write, but this would not catch the case of an inode not reachable
> via path traversal from the callers mount namespace.
> 
> Add check for read-only filesystem to free eofblocks ioctl.
> 
> Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
> ---


looks good to me, thanks

Reviewed-by: Gao feng <gaofeng@cn.fujitsu.com>

>  fs/xfs/xfs_ioctl.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 60d9d1e..c8f3511 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1620,6 +1620,12 @@ xfs_file_ioctl(
>  		struct xfs_fs_eofblocks eofb;
>  		struct xfs_eofblocks keofb;
>  
> +		if (!capable(CAP_SYS_ADMIN))
> +			return -EPERM;
> +
> +		if (mp->m_flags & XFS_MOUNT_RDONLY)
> +			return -XFS_ERROR(EROFS);
> +
>  		if (copy_from_user(&eofb, arg, sizeof(eofb)))
>  			return -XFS_ERROR(EFAULT);
>  
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl
  2013-08-15 18:07 [PATCH v8 0/7] userns: Convert xfs to use kuid_t/kgid_t where appropriate Dwight Engen
@ 2013-08-15 18:08 ` Dwight Engen
  0 siblings, 0 replies; 5+ messages in thread
From: Dwight Engen @ 2013-08-15 18:08 UTC (permalink / raw)
  To: xfs; +Cc: Dwight Engen, Ben Myers

Check for CAP_SYS_ADMIN since the caller can truncate preallocated
blocks from files they do not own nor have write access to. A more
fine grained access check was considered: require the caller to
specify their own uid/gid and to use inode_permission to check for
write, but this would not catch the case of an inode not reachable
via path traversal from the callers mount namespace.

Add check for read-only filesystem to free eofblocks ioctl.

Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Gao feng <gaofeng@cn.fujitsu.com>
Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
---
 fs/xfs/xfs_ioctl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 20b4c7a..bdebc21 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1723,6 +1723,12 @@ xfs_file_ioctl(
 		struct xfs_fs_eofblocks eofb;
 		struct xfs_eofblocks keofb;
 
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+
+		if (mp->m_flags & XFS_MOUNT_RDONLY)
+			return -XFS_ERROR(EROFS);
+
 		if (copy_from_user(&eofb, arg, sizeof(eofb)))
 			return -XFS_ERROR(EFAULT);
 
-- 
1.8.1.4

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-08-15 18:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01 15:30 [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl Dwight Engen
2013-08-01 16:27 ` Brian Foster
2013-08-02  1:47 ` Dave Chinner
2013-08-02  2:47 ` Gao feng
  -- strict thread matches above, loose matches on Subject: below --
2013-08-15 18:07 [PATCH v8 0/7] userns: Convert xfs to use kuid_t/kgid_t where appropriate Dwight Engen
2013-08-15 18:08 ` [PATCH v8 6/7] xfs: add capability check to free eofblocks ioctl Dwight Engen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.