xfs: invalidate cached acl if set directly via xattr
authorBrian Foster <bfoster@redhat.com>
Tue, 3 Nov 2015 01:40:59 +0000 (12:40 +1100)
committerDave Chinner <david@fromorbit.com>
Tue, 3 Nov 2015 01:40:59 +0000 (12:40 +1100)
ACLs are stored as extended attributes of the inode to which they apply.
XFS converts the standard "system.posix_acl_[access|default]" attribute
names used to control ACLs to "trusted.SGI_ACL_[FILE|DEFAULT]" as stored
on-disk. These xattrs are directly exposed in on-disk format via
getxattr/setxattr, without any ACL aware code in the path to perform
validation, etc. This is partly historical and supports backup/restore
applications such as xfsdump to back up and restore the binary blob that
represents ACLs as-is.

Andreas reports that the ACLs observed via the getfacl interface is not
consistent when ACLs are set directly via the setxattr path. This occurs
because the ACLs are cached in-core against the inode and the xattr path
has no knowledge that the operation relates to ACLs.

Update the xattr set codepath to trap writes of the special XFS ACL
attributes and invalidate the associated cached ACL when this occurs.
This ensures that the correct ACLs are used on a subsequent operation
through the actual ACL interface.

Note that this does not update or add support for setting the ACL xattrs
directly beyond the restore use case that requires a correctly formatted
binary blob and to restore a consistent i_mode at the same time. It is
still possible for a root user to set an invalid or inconsistent (with
i_mode) ACL blob on-disk and potentially cause corruption.

[ With fixes from Andreas Gruenbacher. ]

Reported-by: Andreas Gruenbacher <agruenba@redhat.com>
Signed-off-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
fs/xfs/xfs_xattr.c

index c036815183cb69bd82c58a0300c243e7e7ca7f32..1e08d3e85cd041c5d0d6803a11e37c97e7ceaccc 100644 (file)
@@ -57,7 +57,8 @@ static int
 xfs_xattr_set(struct dentry *dentry, const char *name, const void *value,
                size_t size, int flags, int xflags)
 {
-       struct xfs_inode *ip = XFS_I(d_inode(dentry));
+       struct xfs_inode        *ip = XFS_I(d_inode(dentry));
+       int                     error;
 
        if (strcmp(name, "") == 0)
                return -EINVAL;
@@ -70,8 +71,24 @@ xfs_xattr_set(struct dentry *dentry, const char *name, const void *value,
 
        if (!value)
                return xfs_attr_remove(ip, (unsigned char *)name, xflags);
-       return xfs_attr_set(ip, (unsigned char *)name,
+       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.
+        */
+#ifdef CONFIG_XFS_POSIX_ACL
+       if (!error && (xflags & ATTR_ROOT)) {
+               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
+
+       return error;
 }
 
 static const struct xattr_handler xfs_xattr_user_handler = {