From 65654ee7c00bf84e8a4fd9faf32f0459a4335d7e Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 4 Apr 2022 12:38:42 -0700 Subject: [PATCH] Fix getxattr with large values giving EINVAL The change to only allocate a buffer for the first xattr item with kmalloc instead of the entire logical xattr payload with vmalloc included a regression for getting large xattrs. getxattr used to copy the entire payload into the large vmalloc so it could unlock just after get_next_xattr. The change to only getting the first item buffer added a call to copy from the rest of the items but those copies weren't covered by the locks. This would often work because the lock pointer still pointed to a valid lock. But if the lock was invalidated then the mode would no longer be compatible and _item_lookup would return EINVAL. The fix is to extend xattr_rwsem and cluster lock coverage to the rest fo the function body, which includes the value item copies. This also makes getxattr's lock coverage consistent with setxattr and listxattr which might reduce the risk of similar mistakes in the future. Signed-off-by: Zach Brown --- kmod/src/xattr.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kmod/src/xattr.c b/kmod/src/xattr.c index b7623f68..a87b69c2 100644 --- a/kmod/src/xattr.c +++ b/kmod/src/xattr.c @@ -488,28 +488,28 @@ ssize_t scoutfs_getxattr(struct dentry *dentry, const char *name, void *buffer, ret = get_next_xattr(inode, &key, xat, xat_bytes, name, name_len, 0, 0, lck); - up_read(&si->xattr_rwsem); - scoutfs_unlock(sb, lck, SCOUTFS_LOCK_READ); - if (ret < 0) { if (ret == -ENOENT) ret = -ENODATA; - goto out; + goto unlock; } /* the caller just wants to know the size */ if (size == 0) { ret = le16_to_cpu(xat->val_len); - goto out; + goto unlock; } /* the caller's buffer wasn't big enough */ if (size < le16_to_cpu(xat->val_len)) { ret = -ERANGE; - goto out; + goto unlock; } ret = copy_xattr_value(sb, &key, xat, xat_bytes, buffer, size, lck); +unlock: + up_read(&si->xattr_rwsem); + scoutfs_unlock(sb, lck, SCOUTFS_LOCK_READ); out: kfree(xat); return ret;