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

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

[Wenjie Qi]

The user.fadvise xattr handler reads an unsigned int directly from value,
but it is also reached by xattr removal and does not validate the supplied
value length.

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 handler read past the
provided value buffer.

Treat a NULL value as clearing the large-folio inode registration.  Reject
non-NULL user.fadvise values whose length is not exactly
sizeof(unsigned int) before reading the value.

Fixes: 39774f27deaf ("f2fs: another way to set large folio by remembering inode number")
Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
---
 fs/f2fs/xattr.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index 84273936f2a..4e11d774a2c 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -80,10 +80,19 @@ static int f2fs_xattr_generic_get(const struct xattr_handler *handler,
 			     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;
 
+	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 +125,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);
-- 
2.43.0

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

[Chao Yu]

On 5/20/26 22:19, Wenjie Qi wrote:
> The user.fadvise xattr handler reads an unsigned int directly from value,
> but it is also reached by xattr removal and does not validate the supplied
> value length.
> 
> 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 handler read past the
> provided value buffer.
> 
> Treat a NULL value as clearing the large-folio inode registration.  Reject
> non-NULL user.fadvise values whose length is not exactly
> sizeof(unsigned int) before reading the value.
> 

Cc: stable@kernel.org

> Fixes: 39774f27deaf ("f2fs: another way to set large folio by remembering inode number")
> Signed-off-by: Wenjie Qi <qiwenjie@xiaomi.com>
> ---
>   fs/f2fs/xattr.c | 13 +++++++++++--
>   1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index 84273936f2a..4e11d774a2c 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -80,10 +80,19 @@ static int f2fs_xattr_generic_get(const struct xattr_handler *handler,
>   			     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;
>   

Better to add a comment here to describe it's from removexattr("user.fadvise") path?

Thanks,

> +	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 +125,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);