[PATCH v2] f2fs: fix user.fadvise xattr input validation

Wenjie Qi posted 1 patch 3 days, 11 hours ago
fs/f2fs/xattr.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
[PATCH v2] f2fs: fix user.fadvise xattr input validation
Posted by Wenjie Qi 3 days, 11 hours ago
The user.fadvise xattr handlers read and write an unsigned int value,
but neither path validates the full VFS xattr input before using that
synthetic 4-byte value.

removexattr("user.fadvise") calls the xattr set callback with
value == NULL and size == 0, which can dereference NULL.  A normal
setxattr() call with a short value, including size == 0, can also make
the set handler read past the provided value buffer.

The get handler has the same length issue in the other direction.  If
userspace calls getxattr("user.fadvise", buf, 1), VFS allocates a
1-byte kernel buffer and passes that size to the filesystem.  F2FS then
returns 4, causing VFS to copy 4 bytes from that 1-byte buffer back to
userspace.

Treat a NULL value as clearing the large-folio inode registration.
Reject non-NULL user.fadvise set values whose length is not exactly
sizeof(unsigned int), and reject non-NULL get buffers smaller than
sizeof(unsigned int) before using the 4-byte synthetic value.

Fixes: 39774f27deaf ("f2fs: another way to set large folio by remembering inode number")
Cc: stable@kernel.org
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
Changes in v2:
- Add a comment to describe the removexattr("user.fadvise") NULL value path.
- Add Cc: stable@kernel.org.
- Validate the user.fadvise getxattr buffer length as well.

 fs/f2fs/xattr.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 84273936f2a0..48bd25e3d266 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -45,10 +45,13 @@ static void xattr_free(struct f2fs_sb_info *sbi, void *xattr_addr,
 		kfree(xattr_addr);
 }
 
-static int f2fs_xattr_fadvise_get(struct inode *inode, void *buffer)
+static int f2fs_xattr_fadvise_get(struct inode *inode, void *buffer,
+				  size_t size)
 {
 	if (!buffer)
 		goto out;
+	if (size < sizeof(unsigned int))
+		return -ERANGE;
 	if (mapping_large_folio_support(inode->i_mapping))
 		*((unsigned int *)buffer) |= BIT(F2FS_XATTR_FADV_LARGEFOLIO);
 out:
@@ -74,16 +77,26 @@ static int f2fs_xattr_generic_get(const struct xattr_handler *handler,
 	}
 	if (handler->flags == F2FS_XATTR_INDEX_USER &&
 	    !strcmp(name, "fadvise"))
-		return f2fs_xattr_fadvise_get(inode, buffer);
+		return f2fs_xattr_fadvise_get(inode, buffer, size);
 
 	return f2fs_getxattr(inode, handler->flags, name,
 			     buffer, size, NULL);
 }
 
-static int f2fs_xattr_fadvise_set(struct inode *inode, const void *value)
+static int f2fs_xattr_fadvise_set(struct inode *inode, const void *value,
+				  size_t size)
 {
 	unsigned int new_fadvise;
 
+	/* removexattr("user.fadvise") passes NULL to clear the hint. */
+	if (!value) {
+		f2fs_remove_ino_entry(F2FS_I_SB(inode),
+				      inode->i_ino, LARGE_FOLIO_INO);
+		return 0;
+	}
+	if (size != sizeof(new_fadvise))
+		return -EINVAL;
+
 	new_fadvise = *(unsigned int *)value;
 
 	if (new_fadvise & BIT(F2FS_XATTR_FADV_LARGEFOLIO))
@@ -116,7 +129,7 @@ static int f2fs_xattr_generic_set(const struct xattr_handler *handler,
 	}
 	if (handler->flags == F2FS_XATTR_INDEX_USER &&
 	    !strcmp(name, "fadvise"))
-		return f2fs_xattr_fadvise_set(inode, value);
+		return f2fs_xattr_fadvise_set(inode, value, size);
 
 	return f2fs_setxattr(inode, handler->flags, name,
 					value, size, NULL, flags);

base-commit: 520760b9f9156bf9698de38dc44c614fad68a1f9
-- 
2.43.0