* [PATCH v2 1/5] xfs: Validate the length of on-disk ACLs
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
@ 2015-10-30 15:05 ` Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 2/5] xfs: Plug memory leak in xfs_attrmulti_attr_set Andreas Gruenbacher
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-10-30 15:05 UTC (permalink / raw)
To: Dave Chinner, Brian Foster, xfs; +Cc: Andreas Gruenbacher
In xfs_acl_from_disk, instead of trusting that xfs_acl.acl_cnt is correct,
make sure that the length of the attributes is correct as well. Also, turn
the aclp parameter into a const pointer.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/libxfs/xfs_format.h | 8 ++++++--
fs/xfs/xfs_acl.c | 13 ++++++++-----
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
index 9590a06..0e62682 100644
--- a/fs/xfs/libxfs/xfs_format.h
+++ b/fs/xfs/libxfs/xfs_format.h
@@ -1487,9 +1487,13 @@ struct xfs_acl {
sizeof(struct xfs_acl_entry) \
: 25)
-#define XFS_ACL_MAX_SIZE(mp) \
+#define XFS_ACL_SIZE(cnt) \
(sizeof(struct xfs_acl) + \
- sizeof(struct xfs_acl_entry) * XFS_ACL_MAX_ENTRIES((mp)))
+ sizeof(struct xfs_acl_entry) * cnt)
+
+#define XFS_ACL_MAX_SIZE(mp) \
+ XFS_ACL_SIZE(XFS_ACL_MAX_ENTRIES((mp)))
+
/* On-disk XFS extended attribute names */
#define SGI_ACL_FILE "SGI_ACL_FILE"
diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index 4b64167..763e365 100644
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -37,16 +37,19 @@
STATIC struct posix_acl *
xfs_acl_from_disk(
- struct xfs_acl *aclp,
- int max_entries)
+ const struct xfs_acl *aclp,
+ int len,
+ int max_entries)
{
struct posix_acl_entry *acl_e;
struct posix_acl *acl;
- struct xfs_acl_entry *ace;
+ const struct xfs_acl_entry *ace;
unsigned int count, i;
+ if (len < sizeof(*aclp))
+ return ERR_PTR(-EFSCORRUPTED);
count = be32_to_cpu(aclp->acl_cnt);
- if (count > max_entries)
+ if (count > max_entries || XFS_ACL_SIZE(count) != len)
return ERR_PTR(-EFSCORRUPTED);
acl = posix_acl_alloc(count, GFP_KERNEL);
@@ -163,7 +166,7 @@ xfs_get_acl(struct inode *inode, int type)
goto out;
}
- acl = xfs_acl_from_disk(xfs_acl, XFS_ACL_MAX_ENTRIES(ip->i_mount));
+ acl = xfs_acl_from_disk(xfs_acl, len, XFS_ACL_MAX_ENTRIES(ip->i_mount));
if (IS_ERR(acl))
goto out;
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 2/5] xfs: Plug memory leak in xfs_attrmulti_attr_set
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 1/5] xfs: Validate the length of on-disk ACLs Andreas Gruenbacher
@ 2015-10-30 15:05 ` Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 3/5] xfs: SGI ACLs: Fix caching and mode setting Andreas Gruenbacher
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-10-30 15:05 UTC (permalink / raw)
To: Dave Chinner, Brian Foster, xfs; +Cc: Andreas Gruenbacher
When setting attributes via XFS_IOC_ATTRMULTI_BY_HANDLE, the user-space
buffer is copied into a new kernel-space buffer via memdup_user; that
buffer then isn't freed.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/xfs_ioctl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index ea7d85a..e939c20 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -482,6 +482,7 @@ xfs_attrmulti_attr_set(
__uint32_t flags)
{
unsigned char *kbuf;
+ int error;
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
return -EPERM;
@@ -492,7 +493,9 @@ xfs_attrmulti_attr_set(
if (IS_ERR(kbuf))
return PTR_ERR(kbuf);
- return xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);
+ error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);
+ kfree(kbuf);
+ return error;
}
int
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 3/5] xfs: SGI ACLs: Fix caching and mode setting
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 1/5] xfs: Validate the length of on-disk ACLs Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 2/5] xfs: Plug memory leak in xfs_attrmulti_attr_set Andreas Gruenbacher
@ 2015-10-30 15:05 ` Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 4/5] xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers Andreas Gruenbacher
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-10-30 15:05 UTC (permalink / raw)
To: Dave Chinner, Brian Foster, xfs; +Cc: Andreas Gruenbacher
POSIX ACLs on XFS are exposed as system.posix_acl_* as well as
trusted.SGI_ACL_* and via the XFS_IOC_ATTRMULTI_BY_HANDLE ioctl.
Setting the system attributes updates inode->i_mode, inode->i_acl, and
inode->i_default_acl as it should, but setting the trusted attributes
or using the ioctl does not. Fix that by adding xattr handlers for the
two trusted.SGI_ACL_* attributes, and by rerouting the ioctl for these
attributes through the xattr code.
In xfs_xattr_handlers, the new handlers must be installed before the
trusted.* xattr to take effect.
Other than before, the values for those attributes are now verified on
all paths through which they can be set; invalid values are rejected.
Access to the trusted.SGI_ACL_* attributes and to the ioctl is still
limited to users capable of CAP_SYS_ADMIN, while the system.posix_acl_*
attributes can be read by anyone and set by the file owner.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/xfs_acl.c | 105 ++++++++++++++++++++++++++++++++++++++++++++++++++---
fs/xfs/xfs_acl.h | 13 +++++++
fs/xfs/xfs_ioctl.c | 56 +++++++++++++++++++++++++++-
fs/xfs/xfs_xattr.c | 20 +++++++++-
4 files changed, 186 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index 763e365..c094165 100644
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -178,7 +178,7 @@ out:
}
STATIC int
-__xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl)
+___xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl, int xflags)
{
struct xfs_inode *ip = XFS_I(inode);
unsigned char *ea_name;
@@ -212,14 +212,14 @@ __xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl)
(XFS_ACL_MAX_ENTRIES(ip->i_mount) - acl->a_count);
error = xfs_attr_set(ip, ea_name, (unsigned char *)xfs_acl,
- len, ATTR_ROOT);
+ len, xflags);
kmem_free(xfs_acl);
} else {
/*
* A NULL ACL argument means we want to remove the ACL.
*/
- error = xfs_attr_remove(ip, ea_name, ATTR_ROOT);
+ error = xfs_attr_remove(ip, ea_name, xflags);
/*
* If the attribute didn't exist to start with that's fine.
@@ -274,8 +274,9 @@ posix_acl_default_exists(struct inode *inode)
return xfs_acl_exists(inode, SGI_ACL_DEFAULT);
}
-int
-xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
+STATIC int
+__xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type,
+ int xflags)
{
int error = 0;
@@ -303,5 +304,97 @@ xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
}
set_acl:
- return __xfs_set_acl(inode, type, acl);
+ return ___xfs_set_acl(inode, type, acl, xflags);
+}
+
+int
+xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
+{
+ return __xfs_set_acl(inode, acl, type, ATTR_ROOT);
+}
+
+int
+__xfs_xattr_acl_get(struct inode *inode, int type, void *value, size_t size)
+{
+ struct posix_acl *acl;
+ int error;
+
+ if (!IS_POSIXACL(inode))
+ return -EOPNOTSUPP;
+ if (S_ISLNK(inode->i_mode))
+ return -EOPNOTSUPP;
+
+ acl = get_acl(inode, type);
+ if (IS_ERR(acl))
+ return PTR_ERR(acl);
+ if (acl == NULL)
+ return -ENODATA;
+
+ error = XFS_ACL_SIZE(acl->a_count);
+ if (value) {
+ if (error > size)
+ error = -ERANGE;
+ else
+ xfs_acl_to_disk(value, acl);
+ }
+
+ posix_acl_release(acl);
+ return error;
+}
+
+int
+xfs_xattr_acl_get(struct dentry *dentry, const char *name,
+ void *value, size_t size, int type)
+{
+ struct inode *inode = d_inode(dentry);
+
+ if (strcmp(name, "") != 0)
+ return -EINVAL;
+ return __xfs_xattr_acl_get(inode, type, value, size);
+}
+
+int
+__xfs_xattr_acl_set(struct inode *inode, int type, const void *value,
+ size_t size, int xflags)
+{
+ struct xfs_inode *ip = XFS_I(inode);
+ struct posix_acl *acl = NULL;
+ int error;
+
+ if (!IS_POSIXACL(inode))
+ return -EOPNOTSUPP;
+
+ if (value) {
+ acl = xfs_acl_from_disk(value, size, XFS_ACL_MAX_ENTRIES(ip->i_mount));
+ if (IS_ERR(acl))
+ return PTR_ERR(acl);
+
+ if (acl) {
+ error = posix_acl_valid(acl);
+ if (error)
+ goto out;
+ }
+ }
+
+ error = __xfs_set_acl(inode, acl, type, xflags);
+out:
+ posix_acl_release(acl);
+ return error;
+}
+
+int
+xfs_xattr_acl_set(struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags, int type)
+{
+ struct inode *inode = d_inode(dentry);
+ int xflags = ATTR_ROOT;
+
+ if (strcmp(name, "") != 0)
+ return -EINVAL;
+ if (flags & XATTR_CREATE)
+ xflags |= ATTR_CREATE;
+ if (flags & XATTR_REPLACE)
+ xflags |= ATTR_REPLACE;
+
+ return __xfs_xattr_acl_set(inode, type, value, size, xflags);
}
diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h
index 3841b07..22986b6 100644
--- a/fs/xfs/xfs_acl.h
+++ b/fs/xfs/xfs_acl.h
@@ -19,6 +19,7 @@
#define __XFS_ACL_H__
struct inode;
+struct dentry;
struct posix_acl;
struct xfs_inode;
@@ -27,6 +28,18 @@ extern struct posix_acl *xfs_get_acl(struct inode *inode, int type);
extern int xfs_set_acl(struct inode *inode, struct posix_acl *acl, int type);
extern int posix_acl_access_exists(struct inode *inode);
extern int posix_acl_default_exists(struct inode *inode);
+
+extern int __xfs_xattr_acl_get(struct inode *inode, int type, void *value,
+ size_t size);
+extern int __xfs_xattr_acl_set(struct inode *inode, int type, const void *value,
+ size_t size, int xflags);
+
+extern int xfs_xattr_acl_get(struct dentry *dentry, const char *name,
+ void *value, size_t size, int type);
+extern int xfs_xattr_acl_set(struct dentry *dentry, const char *name,
+ const void *value, size_t size, int flags,
+ int type);
+
#else
static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type)
{
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index e939c20..c819dfd 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -40,6 +40,7 @@
#include "xfs_symlink.h"
#include "xfs_trans.h"
#include "xfs_pnfs.h"
+#include "xfs_acl.h"
#include <linux/capability.h>
#include <linux/dcache.h>
@@ -48,6 +49,7 @@
#include <linux/pagemap.h>
#include <linux/slab.h>
#include <linux/exportfs.h>
+#include <linux/posix_acl.h>
/*
* xfs_find_handle maps from userspace xfs_fsop_handlereq structure to
@@ -453,15 +455,39 @@ xfs_attrmulti_attr_get(
__uint32_t flags)
{
unsigned char *kbuf;
- int error = -EFAULT;
+ int error;
if (*len > XATTR_SIZE_MAX)
return -EINVAL;
+
kbuf = kmem_zalloc_large(*len, KM_SLEEP);
if (!kbuf)
return -ENOMEM;
+#ifdef CONFIG_XFS_POSIX_ACL
+ if (flags & ATTR_ROOT) {
+ if (strcmp(name, SGI_ACL_FILE) == 0) {
+ error = __xfs_xattr_acl_get(inode, ACL_TYPE_ACCESS,
+ kbuf, *len);
+ if (error > 0) {
+ *len = error;
+ error = 0;
+ }
+ goto done;
+ } else if (strcmp(name, SGI_ACL_DEFAULT) == 0) {
+ error = __xfs_xattr_acl_get(inode, ACL_TYPE_DEFAULT,
+ kbuf, *len);
+ if (error > 0) {
+ *len = error;
+ error = 0;
+ }
+ goto done;
+ }
+ }
+#endif
+
error = xfs_attr_get(XFS_I(inode), name, kbuf, (int *)len, flags);
+done:
if (error)
goto out_kfree;
@@ -493,7 +519,22 @@ xfs_attrmulti_attr_set(
if (IS_ERR(kbuf))
return PTR_ERR(kbuf);
+#ifdef CONFIG_XFS_POSIX_ACL
+ if (flags & ATTR_ROOT) {
+ if (strcmp(name, SGI_ACL_FILE) == 0) {
+ error = __xfs_xattr_acl_set(inode, ACL_TYPE_ACCESS,
+ kbuf, len, flags);
+ goto out;
+ } else if (strcmp(name, SGI_ACL_DEFAULT) == 0) {
+ error = __xfs_xattr_acl_set(inode, ACL_TYPE_DEFAULT,
+ kbuf, len, flags);
+ goto out;
+ }
+ }
+#endif
+
error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);
+out:
kfree(kbuf);
return error;
}
@@ -506,6 +547,19 @@ xfs_attrmulti_attr_remove(
{
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
return -EPERM;
+
+#ifdef CONFIG_XFS_POSIX_ACL
+ if (flags & ATTR_ROOT) {
+ if (strcmp(name, SGI_ACL_FILE) == 0) {
+ return __xfs_xattr_acl_set(inode, ACL_TYPE_ACCESS,
+ NULL, 0, flags);
+ } else if (strcmp(name, SGI_ACL_DEFAULT) == 0) {
+ return __xfs_xattr_acl_set(inode, ACL_TYPE_DEFAULT,
+ NULL, 0, flags);
+ }
+ }
+#endif
+
return xfs_attr_remove(XFS_I(inode), name, flags);
}
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index c0368151..d8ee7a1 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -95,14 +95,32 @@ static const struct xattr_handler xfs_xattr_security_handler = {
.set = xfs_xattr_set,
};
+#ifdef CONFIG_XFS_POSIX_ACL
+const struct xattr_handler xfs_xattr_sgi_acl_file = {
+ .prefix = XATTR_TRUSTED_PREFIX SGI_ACL_FILE,
+ .flags = ACL_TYPE_ACCESS,
+ .get = xfs_xattr_acl_get,
+ .set = xfs_xattr_acl_set,
+};
+
+const struct xattr_handler xfs_xattr_sgi_acl_default = {
+ .prefix = XATTR_TRUSTED_PREFIX SGI_ACL_DEFAULT,
+ .flags = ACL_TYPE_DEFAULT,
+ .get = xfs_xattr_acl_get,
+ .set = xfs_xattr_acl_set,
+};
+#endif
+
const struct xattr_handler *xfs_xattr_handlers[] = {
&xfs_xattr_user_handler,
- &xfs_xattr_trusted_handler,
&xfs_xattr_security_handler,
#ifdef CONFIG_XFS_POSIX_ACL
&posix_acl_access_xattr_handler,
&posix_acl_default_xattr_handler,
+ &xfs_xattr_sgi_acl_file,
+ &xfs_xattr_sgi_acl_default,
#endif
+ &xfs_xattr_trusted_handler,
NULL
};
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 4/5] xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
` (2 preceding siblings ...)
2015-10-30 15:05 ` [PATCH v2 3/5] xfs: SGI ACLs: Fix caching and mode setting Andreas Gruenbacher
@ 2015-10-30 15:05 ` Andreas Gruenbacher
2015-10-30 15:05 ` [PATCH v2 5/5] xfs: SGI ACLs: Map uid/gid namespaces Andreas Gruenbacher
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
5 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-10-30 15:05 UTC (permalink / raw)
To: Dave Chinner, Brian Foster, xfs; +Cc: Andreas Gruenbacher
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/xfs_acl.c | 8 ++++----
fs/xfs/xfs_inode.c | 14 +++++++-------
fs/xfs/xfs_iops.c | 12 ++++++------
fs/xfs/xfs_linux.h | 21 ++++++++++-----------
fs/xfs/xfs_symlink.c | 4 ++--
5 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index c094165..40fce17 100644
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -71,10 +71,10 @@ xfs_acl_from_disk(
switch (acl_e->e_tag) {
case ACL_USER:
- acl_e->e_uid = xfs_uid_to_kuid(be32_to_cpu(ace->ae_id));
+ acl_e->e_uid = xfs_uid_to_kuid(&init_user_ns, be32_to_cpu(ace->ae_id));
break;
case ACL_GROUP:
- acl_e->e_gid = xfs_gid_to_kgid(be32_to_cpu(ace->ae_id));
+ acl_e->e_gid = xfs_gid_to_kgid(&init_user_ns, be32_to_cpu(ace->ae_id));
break;
case ACL_USER_OBJ:
case ACL_GROUP_OBJ:
@@ -107,10 +107,10 @@ xfs_acl_to_disk(struct xfs_acl *aclp, const struct posix_acl *acl)
ace->ae_tag = cpu_to_be32(acl_e->e_tag);
switch (acl_e->e_tag) {
case ACL_USER:
- ace->ae_id = cpu_to_be32(xfs_kuid_to_uid(acl_e->e_uid));
+ ace->ae_id = cpu_to_be32(xfs_kuid_to_uid(&init_user_ns, acl_e->e_uid));
break;
case ACL_GROUP:
- ace->ae_id = cpu_to_be32(xfs_kgid_to_gid(acl_e->e_gid));
+ ace->ae_id = cpu_to_be32(xfs_kgid_to_gid(&init_user_ns, acl_e->e_gid));
break;
default:
ace->ae_id = cpu_to_be32(ACL_UNDEFINED_ID);
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index dc40a6d..d849b15 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -795,8 +795,8 @@ xfs_ialloc(
ip->i_d.di_onlink = 0;
ip->i_d.di_nlink = nlink;
ASSERT(ip->i_d.di_nlink == nlink);
- ip->i_d.di_uid = xfs_kuid_to_uid(current_fsuid());
- ip->i_d.di_gid = xfs_kgid_to_gid(current_fsgid());
+ ip->i_d.di_uid = xfs_kuid_to_uid(&init_user_ns, current_fsuid());
+ ip->i_d.di_gid = xfs_kgid_to_gid(&init_user_ns, current_fsgid());
xfs_set_projid(ip, prid);
memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
@@ -814,7 +814,7 @@ xfs_ialloc(
*/
if ((irix_sgid_inherit) &&
(ip->i_d.di_mode & S_ISGID) &&
- (!in_group_p(xfs_gid_to_kgid(ip->i_d.di_gid)))) {
+ (!in_group_p(xfs_gid_to_kgid(&init_user_ns, ip->i_d.di_gid)))) {
ip->i_d.di_mode &= ~S_ISGID;
}
@@ -1161,8 +1161,8 @@ xfs_create(
/*
* Make sure that we have allocated dquot(s) on disk.
*/
- error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
- xfs_kgid_to_gid(current_fsgid()), prid,
+ error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(&init_user_ns, current_fsuid()),
+ xfs_kgid_to_gid(&init_user_ns, current_fsgid()), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
@@ -1340,8 +1340,8 @@ xfs_create_tmpfile(
/*
* Make sure that we have allocated dquot(s) on disk.
*/
- error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(current_fsuid()),
- xfs_kgid_to_gid(current_fsgid()), prid,
+ error = xfs_qm_vop_dqalloc(dp, xfs_kuid_to_uid(&init_user_ns, current_fsuid()),
+ xfs_kgid_to_gid(&init_user_ns, current_fsgid()), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index 8294132..f25d2c7 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -598,8 +598,8 @@ xfs_setattr_nonsize(
*/
ASSERT(udqp == NULL);
ASSERT(gdqp == NULL);
- error = xfs_qm_vop_dqalloc(ip, xfs_kuid_to_uid(uid),
- xfs_kgid_to_gid(gid),
+ error = xfs_qm_vop_dqalloc(ip, xfs_kuid_to_uid(&init_user_ns, uid),
+ xfs_kgid_to_gid(&init_user_ns, gid),
xfs_get_projid(ip),
qflags, &udqp, &gdqp, NULL);
if (error)
@@ -671,7 +671,7 @@ xfs_setattr_nonsize(
olddquot1 = xfs_qm_vop_chown(tp, ip,
&ip->i_udquot, udqp);
}
- ip->i_d.di_uid = xfs_kuid_to_uid(uid);
+ ip->i_d.di_uid = xfs_kuid_to_uid(&init_user_ns, uid);
inode->i_uid = uid;
}
if (!gid_eq(igid, gid)) {
@@ -683,7 +683,7 @@ xfs_setattr_nonsize(
olddquot2 = xfs_qm_vop_chown(tp, ip,
&ip->i_gdquot, gdqp);
}
- ip->i_d.di_gid = xfs_kgid_to_gid(gid);
+ ip->i_d.di_gid = xfs_kgid_to_gid(&init_user_ns, gid);
inode->i_gid = gid;
}
}
@@ -1230,8 +1230,8 @@ xfs_setup_inode(
inode->i_mode = ip->i_d.di_mode;
set_nlink(inode, ip->i_d.di_nlink);
- inode->i_uid = xfs_uid_to_kuid(ip->i_d.di_uid);
- inode->i_gid = xfs_gid_to_kgid(ip->i_d.di_gid);
+ inode->i_uid = xfs_uid_to_kuid(&init_user_ns, ip->i_d.di_uid);
+ inode->i_gid = xfs_gid_to_kgid(&init_user_ns, ip->i_d.di_gid);
switch (inode->i_mode & S_IFMT) {
case S_IFBLK:
diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 85f883d..1bf92ec 100644
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -173,28 +173,27 @@ struct xfs_kobj {
/* Kernel uid/gid conversion. These are used to convert to/from the on disk
* uid_t/gid_t types to the kuid_t/kgid_t types that the kernel uses internally.
- * The conversion here is type only, the value will remain the same since we
- * are converting to the init_user_ns. The uid is later mapped to a particular
- * user namespace value when crossing the kernel/user boundary.
+ * The uid is later mapped to a particular user namespace value when crossing
+ * the kernel/user boundary.
*/
-static inline __uint32_t xfs_kuid_to_uid(kuid_t uid)
+static inline __uint32_t xfs_kuid_to_uid(struct user_namespace *ns, kuid_t uid)
{
- return from_kuid(&init_user_ns, uid);
+ return from_kuid(ns, uid);
}
-static inline kuid_t xfs_uid_to_kuid(__uint32_t uid)
+static inline kuid_t xfs_uid_to_kuid(struct user_namespace *ns, __uint32_t uid)
{
- return make_kuid(&init_user_ns, uid);
+ return make_kuid(ns, uid);
}
-static inline __uint32_t xfs_kgid_to_gid(kgid_t gid)
+static inline __uint32_t xfs_kgid_to_gid(struct user_namespace *ns, kgid_t gid)
{
- return from_kgid(&init_user_ns, gid);
+ return from_kgid(ns, gid);
}
-static inline kgid_t xfs_gid_to_kgid(__uint32_t gid)
+static inline kgid_t xfs_gid_to_kgid(struct user_namespace *ns, __uint32_t gid)
{
- return make_kgid(&init_user_ns, gid);
+ return make_kgid(ns, gid);
}
/*
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 996481e..a0c42a9 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -215,8 +215,8 @@ xfs_symlink(
* Make sure that we have allocated dquot(s) on disk.
*/
error = xfs_qm_vop_dqalloc(dp,
- xfs_kuid_to_uid(current_fsuid()),
- xfs_kgid_to_gid(current_fsgid()), prid,
+ xfs_kuid_to_uid(&init_user_ns, current_fsuid()),
+ xfs_kgid_to_gid(&init_user_ns, current_fsgid()), prid,
XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT,
&udqp, &gdqp, &pdqp);
if (error)
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v2 5/5] xfs: SGI ACLs: Map uid/gid namespaces
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
` (3 preceding siblings ...)
2015-10-30 15:05 ` [PATCH v2 4/5] xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers Andreas Gruenbacher
@ 2015-10-30 15:05 ` Andreas Gruenbacher
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
5 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-10-30 15:05 UTC (permalink / raw)
To: Dave Chinner, Brian Foster, xfs; +Cc: Andreas Gruenbacher
Map uids and gids in the trusted.SGI_ACL_{FILE,DEFAULT} attributes between
the kernel and user-space namespaces. This needs to be done in the
filesystem because the VFS is unaware of those attributes; for the standard
POSIX ACL attributes, the VFS takes care of that for us.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/xfs_acl.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
index 40fce17..7076d07 100644
--- a/fs/xfs/xfs_acl.c
+++ b/fs/xfs/xfs_acl.c
@@ -39,7 +39,8 @@ STATIC struct posix_acl *
xfs_acl_from_disk(
const struct xfs_acl *aclp,
int len,
- int max_entries)
+ int max_entries,
+ struct user_namespace *ns)
{
struct posix_acl_entry *acl_e;
struct posix_acl *acl;
@@ -71,10 +72,10 @@ xfs_acl_from_disk(
switch (acl_e->e_tag) {
case ACL_USER:
- acl_e->e_uid = xfs_uid_to_kuid(&init_user_ns, be32_to_cpu(ace->ae_id));
+ acl_e->e_uid = xfs_uid_to_kuid(ns, be32_to_cpu(ace->ae_id));
break;
case ACL_GROUP:
- acl_e->e_gid = xfs_gid_to_kgid(&init_user_ns, be32_to_cpu(ace->ae_id));
+ acl_e->e_gid = xfs_gid_to_kgid(ns, be32_to_cpu(ace->ae_id));
break;
case ACL_USER_OBJ:
case ACL_GROUP_OBJ:
@@ -93,7 +94,10 @@ fail:
}
STATIC void
-xfs_acl_to_disk(struct xfs_acl *aclp, const struct posix_acl *acl)
+xfs_acl_to_disk(
+ struct xfs_acl *aclp,
+ const struct posix_acl *acl,
+ struct user_namespace *ns)
{
const struct posix_acl_entry *acl_e;
struct xfs_acl_entry *ace;
@@ -107,10 +111,10 @@ xfs_acl_to_disk(struct xfs_acl *aclp, const struct posix_acl *acl)
ace->ae_tag = cpu_to_be32(acl_e->e_tag);
switch (acl_e->e_tag) {
case ACL_USER:
- ace->ae_id = cpu_to_be32(xfs_kuid_to_uid(&init_user_ns, acl_e->e_uid));
+ ace->ae_id = cpu_to_be32(xfs_kuid_to_uid(ns, acl_e->e_uid));
break;
case ACL_GROUP:
- ace->ae_id = cpu_to_be32(xfs_kgid_to_gid(&init_user_ns, acl_e->e_gid));
+ ace->ae_id = cpu_to_be32(xfs_kgid_to_gid(ns, acl_e->e_gid));
break;
default:
ace->ae_id = cpu_to_be32(ACL_UNDEFINED_ID);
@@ -166,7 +170,8 @@ xfs_get_acl(struct inode *inode, int type)
goto out;
}
- acl = xfs_acl_from_disk(xfs_acl, len, XFS_ACL_MAX_ENTRIES(ip->i_mount));
+ acl = xfs_acl_from_disk(xfs_acl, len, XFS_ACL_MAX_ENTRIES(ip->i_mount),
+ &init_user_ns);
if (IS_ERR(acl))
goto out;
@@ -205,7 +210,7 @@ ___xfs_set_acl(struct inode *inode, int type, struct posix_acl *acl, int xflags)
if (!xfs_acl)
return -ENOMEM;
- xfs_acl_to_disk(xfs_acl, acl);
+ xfs_acl_to_disk(xfs_acl, acl, &init_user_ns);
/* subtract away the unused acl entries */
len -= sizeof(struct xfs_acl_entry) *
@@ -332,10 +337,11 @@ __xfs_xattr_acl_get(struct inode *inode, int type, void *value, size_t size)
error = XFS_ACL_SIZE(acl->a_count);
if (value) {
+ struct user_namespace *user_ns = current_user_ns();
if (error > size)
error = -ERANGE;
else
- xfs_acl_to_disk(value, acl);
+ xfs_acl_to_disk(value, acl, user_ns);
}
posix_acl_release(acl);
@@ -365,7 +371,10 @@ __xfs_xattr_acl_set(struct inode *inode, int type, const void *value,
return -EOPNOTSUPP;
if (value) {
- acl = xfs_acl_from_disk(value, size, XFS_ACL_MAX_ENTRIES(ip->i_mount));
+ struct user_namespace *user_ns = current_user_ns();
+ acl = xfs_acl_from_disk(value, size,
+ XFS_ACL_MAX_ENTRIES(ip->i_mount),
+ user_ns);
if (IS_ERR(acl))
return PTR_ERR(acl);
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] xfs: SGI ACL Fixes
2015-10-30 15:05 [PATCH v2 0/5] xfs: SGI ACL Fixes Andreas Gruenbacher
` (4 preceding siblings ...)
2015-10-30 15:05 ` [PATCH v2 5/5] xfs: SGI ACLs: Map uid/gid namespaces Andreas Gruenbacher
@ 2015-11-02 2:53 ` Dave Chinner
2015-11-02 3:41 ` Andreas Gruenbacher
` (3 more replies)
5 siblings, 4 replies; 13+ messages in thread
From: Dave Chinner @ 2015-11-02 2:53 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: Brian Foster, xfs
On Fri, Oct 30, 2015 at 04:05:03PM +0100, Andreas Gruenbacher wrote:
> Here is a reworked patch queue that also handles setting SGI_ACL_{FILE,DEFAULT}
> via XFS_IOC_ATTRMULTI_BY_HANDLE. Please review.
>
> Thanks,
> Andreas
>
> Andreas Gruenbacher (5):
> xfs: Validate the length of on-disk ACLs
> xfs: Plug memory leak in xfs_attrmulti_attr_set
Ok, I've taken these two patches for the upcoming merge window as
they fix bugs, but I've taken Brian's cached ACL invalidation patch
instead of these:
> xfs: SGI ACLs: Fix caching and mode setting
> xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers
> xfs: SGI ACLs: Map uid/gid namespaces
I'm not yet convinced that these patches are the right way to solve
the given issue as they may interact badly with xfsdump/restore.
However, I do want the kernel code to behave correctly after
xfs_restore runs and Brian's change is enough to do this. If
we do decide that we need to make the above changes to the posix
acl code, it's easy enough to replace this invalidation code.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] xfs: SGI ACL Fixes
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
@ 2015-11-02 3:41 ` Andreas Gruenbacher
2015-11-02 12:20 ` Dave Chinner
2015-11-02 19:52 ` Andreas Gruenbacher
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-11-02 3:41 UTC (permalink / raw)
To: Dave Chinner; +Cc: Brian Foster, xfs
On Mon, Nov 2, 2015 at 3:53 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Oct 30, 2015 at 04:05:03PM +0100, Andreas Gruenbacher wrote:
>> Here is a reworked patch queue that also handles setting SGI_ACL_{FILE,DEFAULT}
>> via XFS_IOC_ATTRMULTI_BY_HANDLE. Please review.
>>
>> Thanks,
>> Andreas
>>
>> Andreas Gruenbacher (5):
>> xfs: Validate the length of on-disk ACLs
>> xfs: Plug memory leak in xfs_attrmulti_attr_set
>
> Ok, I've taken these two patches for the upcoming merge window as
> they fix bugs, but I've taken Brian's cached ACL invalidation patch
> instead of these:
>
>> xfs: SGI ACLs: Fix caching and mode setting
>> xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers
>> xfs: SGI ACLs: Map uid/gid namespaces
>
> I'm not yet convinced that these patches are the right way to solve
> the given issue as they may interact badly with xfsdump/restore.
Well, what else do you need to be convinced? I guess it won't help if
I try to explain everything all over again?
Thanks,
Andreas
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2 0/5] xfs: SGI ACL Fixes
2015-11-02 3:41 ` Andreas Gruenbacher
@ 2015-11-02 12:20 ` Dave Chinner
0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2015-11-02 12:20 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: Brian Foster, xfs
On Mon, Nov 02, 2015 at 04:41:30AM +0100, Andreas Gruenbacher wrote:
> On Mon, Nov 2, 2015 at 3:53 AM, Dave Chinner <david@fromorbit.com> wrote:
> > On Fri, Oct 30, 2015 at 04:05:03PM +0100, Andreas Gruenbacher wrote:
> >> Here is a reworked patch queue that also handles setting SGI_ACL_{FILE,DEFAULT}
> >> via XFS_IOC_ATTRMULTI_BY_HANDLE. Please review.
> >>
> >> Thanks,
> >> Andreas
> >>
> >> Andreas Gruenbacher (5):
> >> xfs: Validate the length of on-disk ACLs
> >> xfs: Plug memory leak in xfs_attrmulti_attr_set
> >
> > Ok, I've taken these two patches for the upcoming merge window as
> > they fix bugs, but I've taken Brian's cached ACL invalidation patch
> > instead of these:
> >
> >> xfs: SGI ACLs: Fix caching and mode setting
> >> xfs: Add namespace parameter to the xfs kuid/kgid <=> uid/gid wrappers
> >> xfs: SGI ACLs: Map uid/gid namespaces
> >
> > I'm not yet convinced that these patches are the right way to solve
> > the given issue as they may interact badly with xfsdump/restore.
>
> Well, what else do you need to be convinced? I guess it won't help if
> I try to explain everything all over again?
No, it won't. There are bigger picture questions and issues that
need to be sorted out first.
We've basically got zero regression test coverage of the "set the
ACL in on-disk format directly" code path, so we don't know if we're
breaking anything. That's a big red flag to me, because I can't
easily validate that the changes are working as expected..
IOWs, I don't know how the changes will impact dump/restore, apart
from the fact that dump/restore expects to be able to set the ACL
directly in the on-disk format without the kernel screwing with the
mode. i.e. dump pulls the inode mode and ACLs in disk format from
the disk via bulkstat and restore expects to be able to recreate
them exactly without the kernel getting in it's way. Again, we have
basically no test coverage here, so again there's nothing I can do
to easily validate that the changes have not broken dump/restore in
subtle but important ways.
Further, we have no user namespace regression test coverage. I have
absolutely no idea if xfs_restore will even run in a user namespace,
let alone run correctly. We already know that xfsdump cannot be
made to work in a confined user namespace due to it's use of
bulkstat, so do we really care whether xfs_restore works in a
namespace or not? As such, it's not immediately obvious to me that
we should even allow setting acls directly in on-disk format in
user namespaces. And if we don't allow it, then the last two patches
can simply be dropped and replaced with simple CAP_SYS_ADMIN check.
I don't have time to sit down and write special point tests to
validate every change that comes through. However, ACLs are a security
mechanism and so we have to get them right and that means we need
such tests. If you want to convince me that your changes are
correct, are needed and don't break anything, then provide the
regression tests that prove it. Otherwise you're simply going to
have to wait for me to find the time to do it myself - I've got a
real long list of stuff I'd prefer to do than write ACL regression
tests....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/5] xfs: SGI ACL Fixes
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
2015-11-02 3:41 ` Andreas Gruenbacher
@ 2015-11-02 19:52 ` Andreas Gruenbacher
2015-11-02 19:52 ` [PATCH 1/2] xfs: Fixes to "invalidate cached acl if set directly via xattr" Andreas Gruenbacher
2015-11-02 19:52 ` [PATCH 2/2] xfs: invalidate cached acl if set via ioctl Andreas Gruenbacher
3 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-11-02 19:52 UTC (permalink / raw)
To: Dave Chinner; +Cc: Brian Foster, Andreas Gruenbacher, xfs
On Mon, Nov 2, 2015 at 3:53 AM, Dave Chinner <david@fromorbit.com> wrote:
> Ok, I've taken these two patches for the upcoming merge window as
> they fix bugs, but I've taken Brian's cached ACL invalidation patch
> instead [...]
Then you'll also need these two patches.
Andreas
Andreas Gruenbacher (2):
xfs: Fixes to "invalidate cached acl if set directly via xattr"
xfs: invalidate cached acl if set via ioctl
fs/xfs/xfs_acl.h | 3 +++
fs/xfs/xfs_ioctl.c | 10 +++++++++-
fs/xfs/xfs_xattr.c | 36 ++++++++++++++++++++++++------------
3 files changed, 36 insertions(+), 13 deletions(-)
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] xfs: Fixes to "invalidate cached acl if set directly via xattr"
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
2015-11-02 3:41 ` Andreas Gruenbacher
2015-11-02 19:52 ` Andreas Gruenbacher
@ 2015-11-02 19:52 ` Andreas Gruenbacher
2015-11-02 19:52 ` [PATCH 2/2] xfs: invalidate cached acl if set via ioctl Andreas Gruenbacher
3 siblings, 0 replies; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-11-02 19:52 UTC (permalink / raw)
To: Dave Chinner; +Cc: Brian Foster, Andreas Gruenbacher, Andreas Gruenbacher, xfs
Don't match arbitrary name prefixes like "S" when checking for the
attribute names "SGI_ACL_{FILE,DEFAULT}".
Function forget_cached_acl only exists in kernels that have POSIX ACL
support; guard calls to that function by CONFIG_XFS_POSIX_ACL.
Signed-off-by: Andreas Gruenbacher <agruenba@kernel.org>
---
fs/xfs/xfs_xattr.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 2e1eb80..1542d64 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -80,10 +80,12 @@ xfs_xattr_set(struct dentry *dentry, const char *name, const void *value,
* consistent.
*/
if (!error && (xflags & ATTR_ROOT)) {
- if (!strncmp(name, SGI_ACL_FILE, strlen(name)))
+#ifdef CONFIG_XFS_POSIX_ACL
+ if (!strcmp(name, SGI_ACL_FILE)
forget_cached_acl(VFS_I(ip), ACL_TYPE_ACCESS);
- else if (!strncmp(name, SGI_ACL_DEFAULT, strlen(name)))
+ else if (!strcmp(name, SGI_ACL_DEFAULT))
forget_cached_acl(VFS_I(ip), ACL_TYPE_DEFAULT);
+#endif
}
return error;
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/2] xfs: invalidate cached acl if set via ioctl
2015-11-02 2:53 ` [PATCH v2 0/5] xfs: SGI ACL Fixes Dave Chinner
` (2 preceding siblings ...)
2015-11-02 19:52 ` [PATCH 1/2] xfs: Fixes to "invalidate cached acl if set directly via xattr" Andreas Gruenbacher
@ 2015-11-02 19:52 ` Andreas Gruenbacher
2015-11-03 2:12 ` Dave Chinner
3 siblings, 1 reply; 13+ messages in thread
From: Andreas Gruenbacher @ 2015-11-02 19:52 UTC (permalink / raw)
To: Dave Chinner; +Cc: Brian Foster, Andreas Gruenbacher, xfs
Setting or removing the "SGI_ACL_[FILE|DEFAULT]" attributes via the
XFS_IOC_ATTRMULTI_BY_HANDLE ioctl completely bypasses the POSIX ACL
infrastructure, like setting the "trusted.SGI_ACL_[FILE|DEFAULT]" xattrs
did until commit 6caa1056. Similar to that commit, invalidate cached
acls when setting/removing them via the ioctl as well.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/xfs/xfs_acl.h | 3 +++
fs/xfs/xfs_ioctl.c | 10 +++++++++-
fs/xfs/xfs_xattr.c | 38 ++++++++++++++++++++++++--------------
3 files changed, 36 insertions(+), 15 deletions(-)
diff --git a/fs/xfs/xfs_acl.h b/fs/xfs/xfs_acl.h
index 3841b07..75af0a4 100644
--- a/fs/xfs/xfs_acl.h
+++ b/fs/xfs/xfs_acl.h
@@ -36,4 +36,7 @@ static inline struct posix_acl *xfs_get_acl(struct inode *inode, int type)
# define posix_acl_access_exists(inode) 0
# define posix_acl_default_exists(inode) 0
#endif /* CONFIG_XFS_POSIX_ACL */
+
+extern void xfs_forget_acl(struct inode *inode, const char *name, int xflags);
+
#endif /* __XFS_ACL_H__ */
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 66bcfbd..d42738d 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -40,6 +40,7 @@
#include "xfs_symlink.h"
#include "xfs_trans.h"
#include "xfs_pnfs.h"
+#include "xfs_acl.h"
#include <linux/capability.h>
#include <linux/dcache.h>
@@ -494,6 +495,8 @@ xfs_attrmulti_attr_set(
return PTR_ERR(kbuf);
error = xfs_attr_set(XFS_I(inode), name, kbuf, len, flags);
+ if (!error)
+ xfs_forget_acl(inode, name, flags);
kfree(kbuf);
return error;
}
@@ -504,9 +507,14 @@ xfs_attrmulti_attr_remove(
unsigned char *name,
__uint32_t flags)
{
+ int error;
+
if (IS_IMMUTABLE(inode) || IS_APPEND(inode))
return -EPERM;
- return xfs_attr_remove(XFS_I(inode), name, flags);
+ error = xfs_attr_remove(XFS_I(inode), name, flags);
+ if (!error)
+ xfs_forget_acl(inode, name, flags);
+ return error;
}
STATIC int
diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
index 1542d64..8294f86 100644
--- a/fs/xfs/xfs_xattr.c
+++ b/fs/xfs/xfs_xattr.c
@@ -53,6 +53,28 @@ xfs_xattr_get(struct dentry *dentry, const char *name,
return asize;
}
+void
+xfs_forget_acl(
+ struct inode *inode,
+ const char *name,
+ int xflags)
+{
+ /*
+ * Invalidate any cached ACLs if the user has bypassed the ACL
+ * interface. We don't validate the content whatsoever so it is caller
+ * responsibility to provide data in valid format and ensure i_mode is
+ * consistent.
+ */
+ if (xflags & ATTR_ROOT) {
+#ifdef CONFIG_XFS_POSIX_ACL
+ if (!strcmp(name, SGI_ACL_FILE))
+ forget_cached_acl(inode, ACL_TYPE_ACCESS);
+ else if (!strcmp(name, SGI_ACL_DEFAULT))
+ forget_cached_acl(inode, ACL_TYPE_DEFAULT);
+#endif
+ }
+}
+
static int
xfs_xattr_set(struct dentry *dentry, const char *name, const void *value,
size_t size, int flags, int xflags)
@@ -73,20 +95,8 @@ xfs_xattr_set(struct dentry *dentry, const char *name, const void *value,
return xfs_attr_remove(ip, (unsigned char *)name, xflags);
error = xfs_attr_set(ip, (unsigned char *)name,
(void *)value, size, xflags);
- /*
- * Invalidate any cached ACLs if the user has bypassed the ACL
- * interface. We don't validate the content whatsoever so it is caller
- * responsibility to provide data in valid format and ensure i_mode is
- * consistent.
- */
- if (!error && (xflags & ATTR_ROOT)) {
-#ifdef CONFIG_XFS_POSIX_ACL
- if (!strcmp(name, SGI_ACL_FILE)
- forget_cached_acl(VFS_I(ip), ACL_TYPE_ACCESS);
- else if (!strcmp(name, SGI_ACL_DEFAULT))
- forget_cached_acl(VFS_I(ip), ACL_TYPE_DEFAULT);
-#endif
- }
+ if (!error)
+ xfs_forget_acl(d_inode(dentry), name, xflags);
return error;
}
--
2.5.0
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/2] xfs: invalidate cached acl if set via ioctl
2015-11-02 19:52 ` [PATCH 2/2] xfs: invalidate cached acl if set via ioctl Andreas Gruenbacher
@ 2015-11-03 2:12 ` Dave Chinner
0 siblings, 0 replies; 13+ messages in thread
From: Dave Chinner @ 2015-11-03 2:12 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: Brian Foster, xfs
On Mon, Nov 02, 2015 at 08:52:54PM +0100, Andreas Gruenbacher wrote:
> Setting or removing the "SGI_ACL_[FILE|DEFAULT]" attributes via the
> XFS_IOC_ATTRMULTI_BY_HANDLE ioctl completely bypasses the POSIX ACL
> infrastructure, like setting the "trusted.SGI_ACL_[FILE|DEFAULT]" xattrs
> did until commit 6caa1056. Similar to that commit, invalidate cached
> acls when setting/removing them via the ioctl as well.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Thanks, Andreas. I've pulled the fixes in. I updated the original
patch with the fix in patch 1, and applied this patch. I'll push out
a rebased for-next branch after some further testing...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread