All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace
@ 2013-08-01 15:30 Dwight Engen
  2013-08-02  1:46 ` Dave Chinner
  2013-08-02  2:47 ` Gao feng
  0 siblings, 2 replies; 4+ messages in thread
From: Dwight Engen @ 2013-08-01 15:30 UTC (permalink / raw)
  To: xfs

Use inode_capable() to check if SUID|SGID bits should be cleared to match
similar check in inode_change_ok().

The check for CAP_LINUX_IMMUTABLE was not modified since all other file
systems also check against init_user_ns rather than current_user_ns.

Only allow changing of projid from init_user_ns.

Signed-off-by: Dwight Engen <dwight.engen@oracle.com>
---
 fs/xfs/xfs_ioctl.c  | 11 +++++++++--
 kernel/capability.c |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 6e2bca5..7b42192 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -981,15 +981,22 @@ xfs_ioctl_setattr(
 	 * to the file owner ID, except in cases where the
 	 * CAP_FSETID capability is applicable.
 	 */
-	if (current_fsuid() != ip->i_d.di_uid && !capable(CAP_FOWNER)) {
+	if (!inode_owner_or_capable(VFS_I(ip))) {
 		code = XFS_ERROR(EPERM);
 		goto error_return;
 	}
 
 	/*
 	 * Do a quota reservation only if projid is actually going to change.
+	 * Only allow changing of projid from init_user_ns since it is a
+	 * non user namespace aware identifier.
 	 */
 	if (mask & FSX_PROJID) {
+		if (current_user_ns() != &init_user_ns) {
+			code = XFS_ERROR(EINVAL);
+			goto error_return;
+		}
+
 		if (XFS_IS_QUOTA_RUNNING(mp) &&
 		    XFS_IS_PQUOTA_ON(mp) &&
 		    xfs_get_projid(ip) != fa->fsx_projid) {
@@ -1103,7 +1110,7 @@ xfs_ioctl_setattr(
 		 * cleared upon successful return from chown()
 		 */
 		if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
-		    !capable(CAP_FSETID))
+		    !inode_capable(VFS_I(ip), CAP_FSETID))
 			ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
 
 		/*
diff --git a/kernel/capability.c b/kernel/capability.c
index f6c2ce5..a4b6744 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -464,3 +464,4 @@ bool inode_capable(const struct inode *inode, int cap)
 
 	return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid);
 }
+EXPORT_SYMBOL(inode_capable);
-- 
1.8.1.4

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace
  2013-08-01 15:30 [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace Dwight Engen
@ 2013-08-02  1:46 ` Dave Chinner
  2013-08-02  2:47 ` Gao feng
  1 sibling, 0 replies; 4+ messages in thread
From: Dave Chinner @ 2013-08-02  1:46 UTC (permalink / raw)
  To: Dwight Engen; +Cc: xfs

On Thu, Aug 01, 2013 at 11:30:06AM -0400, Dwight Engen wrote:
> Use inode_capable() to check if SUID|SGID bits should be cleared to match
> similar check in inode_change_ok().
> 
> The check for CAP_LINUX_IMMUTABLE was not modified since all other file
> systems also check against init_user_ns rather than current_user_ns.
> 
> Only allow changing of projid from init_user_ns.
> 
> 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] 4+ messages in thread
* Re: [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace
  2013-08-01 15:30 [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace Dwight Engen
  2013-08-02  1:46 ` Dave Chinner
@ 2013-08-02  2:47 ` Gao feng
  1 sibling, 0 replies; 4+ 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:
> Use inode_capable() to check if SUID|SGID bits should be cleared to match
> similar check in inode_change_ok().
> 
> The check for CAP_LINUX_IMMUTABLE was not modified since all other file
> systems also check against init_user_ns rather than current_user_ns.
> 
> Only allow changing of projid from init_user_ns.
> 
> 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  | 11 +++++++++--
>  kernel/capability.c |  1 +
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 6e2bca5..7b42192 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -981,15 +981,22 @@ xfs_ioctl_setattr(
>  	 * to the file owner ID, except in cases where the
>  	 * CAP_FSETID capability is applicable.
>  	 */
> -	if (current_fsuid() != ip->i_d.di_uid && !capable(CAP_FOWNER)) {
> +	if (!inode_owner_or_capable(VFS_I(ip))) {
>  		code = XFS_ERROR(EPERM);
>  		goto error_return;
>  	}
>  
>  	/*
>  	 * Do a quota reservation only if projid is actually going to change.
> +	 * Only allow changing of projid from init_user_ns since it is a
> +	 * non user namespace aware identifier.
>  	 */
>  	if (mask & FSX_PROJID) {
> +		if (current_user_ns() != &init_user_ns) {
> +			code = XFS_ERROR(EINVAL);
> +			goto error_return;
> +		}
> +
>  		if (XFS_IS_QUOTA_RUNNING(mp) &&
>  		    XFS_IS_PQUOTA_ON(mp) &&
>  		    xfs_get_projid(ip) != fa->fsx_projid) {
> @@ -1103,7 +1110,7 @@ xfs_ioctl_setattr(
>  		 * cleared upon successful return from chown()
>  		 */
>  		if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
> -		    !capable(CAP_FSETID))
> +		    !inode_capable(VFS_I(ip), CAP_FSETID))
>  			ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
>  
>  		/*
> diff --git a/kernel/capability.c b/kernel/capability.c
> index f6c2ce5..a4b6744 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -464,3 +464,4 @@ bool inode_capable(const struct inode *inode, int cap)
>  
>  	return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid);
>  }
> +EXPORT_SYMBOL(inode_capable);
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace
  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; 4+ messages in thread
From: Dwight Engen @ 2013-08-15 18:08 UTC (permalink / raw)
  To: xfs; +Cc: Dwight Engen, Ben Myers

Use inode_capable() to check if SUID|SGID bits should be cleared to match
similar check in inode_change_ok().

The check for CAP_LINUX_IMMUTABLE was not modified since all other file
systems also check against init_user_ns rather than current_user_ns.

Only allow changing of projid from init_user_ns.

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  | 11 +++++++++--
 kernel/capability.c |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index e9c17e2..999c1ef 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -1015,15 +1015,22 @@ xfs_ioctl_setattr(
 	 * to the file owner ID, except in cases where the
 	 * CAP_FSETID capability is applicable.
 	 */
-	if (current_fsuid() != ip->i_d.di_uid && !capable(CAP_FOWNER)) {
+	if (!inode_owner_or_capable(VFS_I(ip))) {
 		code = XFS_ERROR(EPERM);
 		goto error_return;
 	}
 
 	/*
 	 * Do a quota reservation only if projid is actually going to change.
+	 * Only allow changing of projid from init_user_ns since it is a
+	 * non user namespace aware identifier.
 	 */
 	if (mask & FSX_PROJID) {
+		if (current_user_ns() != &init_user_ns) {
+			code = XFS_ERROR(EINVAL);
+			goto error_return;
+		}
+
 		if (XFS_IS_QUOTA_RUNNING(mp) &&
 		    XFS_IS_PQUOTA_ON(mp) &&
 		    xfs_get_projid(ip) != fa->fsx_projid) {
@@ -1137,7 +1144,7 @@ xfs_ioctl_setattr(
 		 * cleared upon successful return from chown()
 		 */
 		if ((ip->i_d.di_mode & (S_ISUID|S_ISGID)) &&
-		    !capable(CAP_FSETID))
+		    !inode_capable(VFS_I(ip), CAP_FSETID))
 			ip->i_d.di_mode &= ~(S_ISUID|S_ISGID);
 
 		/*
diff --git a/kernel/capability.c b/kernel/capability.c
index f6c2ce5..a4b6744 100644
--- a/kernel/capability.c
+++ b/kernel/capability.c
@@ -464,3 +464,4 @@ bool inode_capable(const struct inode *inode, int cap)
 
 	return ns_capable(ns, cap) && kuid_has_mapping(ns, inode->i_uid);
 }
+EXPORT_SYMBOL(inode_capable);
-- 
1.8.1.4

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-08-15 18:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-01 15:30 [PATCH v8 3/7] xfs: ioctl check for capabilities in the current user namespace Dwight Engen
2013-08-02  1:46 ` 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 3/7] xfs: ioctl check for capabilities in the current user namespace 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.