[v1] 9p: bound the xattr size used to allocate the ACL buffer

[PATCH] 9p: bound the xattr size used to allocate the ACL buffer

Posted by Nguyen Ngoc Thang 5 days, 11 hours ago

v9fs_fid_get_acl() sizes its kzalloc() buffer from the xattr length
reported by the 9p server. A server, or a syzbot-style fake one, can
report an arbitrarily large value. When it exceeds what the page
allocator can serve, mounting with posixacl,access=client trips:

  WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof
   ___kmalloc_large_node
   v9fs_fid_get_acl
   v9fs_get_acl
   v9fs_inode_from_fid_dotl
   v9fs_get_tree

A valid POSIX ACL always fits in an xattr value, so reject sizes above
XATTR_SIZE_MAX. __v9fs_get_acl() already maps this error to -EIO.

Reported-by: syzbot+de6fd6789748a8aa64a0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de6fd6789748a8aa64a0
Fixes: 85ff872d3f4a ("fs/9p: Implement POSIX ACL permission checking function")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
 fs/9p/acl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index ae7e7cf7523a..0dd7e72bbdd3 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -29,6 +29,9 @@ static struct posix_acl *v9fs_fid_get_acl(struct p9_fid *fid, const char *name)
 		return ERR_PTR(size);
 	if (size == 0)
 		return ERR_PTR(-ENODATA);
+	/* the size is server-controlled; a valid ACL fits in an xattr */
+	if (size > XATTR_SIZE_MAX)
+		return ERR_PTR(-E2BIG);
 
 	value = kzalloc(size, GFP_NOFS);
 	if (!value)
-- 
2.43.0

Re: [PATCH] 9p: bound the xattr size used to allocate the ACL buffer

Posted by Christian Schoenebeck 5 days, 8 hours ago

On Saturday, 19 September 2026 12:29:58 CEST Nguyen Ngoc Thang wrote:
> v9fs_fid_get_acl() sizes its kzalloc() buffer from the xattr length
> reported by the 9p server. A server, or a syzbot-style fake one, can
> report an arbitrarily large value. When it exceeds what the page
> allocator can serve, mounting with posixacl,access=client trips:
> 
>   WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof
>    ___kmalloc_large_node
>    v9fs_fid_get_acl
>    v9fs_get_acl
>    v9fs_inode_from_fid_dotl
>    v9fs_get_tree
> 
> A valid POSIX ACL always fits in an xattr value, so reject sizes above
> XATTR_SIZE_MAX. __v9fs_get_acl() already maps this error to -EIO.

What it currently does is calling kzalloc() and if that fails, it remaps the 
error to -EIO. However the Linux ACL subsystem does currently not impose its 
own limit, and therefore ACL size > XATTR_SIZE_MAX doesn't necessarily render 
it invalid, at least not if 9p server supports larger (9p) xattrs.

> Reported-by: syzbot+de6fd6789748a8aa64a0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=de6fd6789748a8aa64a0
> Fixes: 85ff872d3f4a ("fs/9p: Implement POSIX ACL permission checking
> function") Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
>  fs/9p/acl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index ae7e7cf7523a..0dd7e72bbdd3 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -29,6 +29,9 @@ static struct posix_acl *v9fs_fid_get_acl(struct p9_fid
> *fid, const char *name) return ERR_PTR(size);
>  	if (size == 0)
>  		return ERR_PTR(-ENODATA);
> +	/* the size is server-controlled; a valid ACL fits in an xattr */
> +	if (size > XATTR_SIZE_MAX)
> +		return ERR_PTR(-E2BIG);

We already had related discussions about this before (without decision), i.e. 
XATTR_SIZE_MAX vs. KMALLOC_MAX_SIZE. Note the source location was different, 
but it is the same issue:

https://lore.kernel.org/all/Z1n-Ue19Pa_AWVu0@codewreck.org/

XATTR_SIZE_MAX < KMALLOC_MAX_SIZE

KMALLOC_MAX_SIZE is currently (already) the real effective upper bound, and 
therefore catching KMALLOC_MAX_SIZE here would basically just silence syzbot 
reports. The only actual reason to address this issue at all.

Capping to XATTR_SIZE_MAX here would make sense, assuming 9p server is a Linux 
system as well. But it may not be. So for non-Linux 9p servers (and for Linux 
9p servers that do not map 9p xattrs to filesystem xattrs) you would 
unnecessarily lower the size limit for ACLs, and ACLs can be huge.

> 
>  	value = kzalloc(size, GFP_NOFS);
>  	if (!value)

[PATCH v2] 9p: bound the xattr size used to allocate the ACL buffer

Posted by Nguyen Ngoc Thang 5 days, 8 hours ago

v9fs_fid_get_acl() sizes its kzalloc() buffer from the xattr length
reported by the 9p server. A server, or a syzbot-style fake one, can
report an arbitrarily large value. When it exceeds what the page
allocator can serve, mounting with posixacl,access=client trips:

  WARNING: mm/page_alloc.c:5340 at __alloc_frozen_pages_noprof
   ___kmalloc_large_node
   v9fs_fid_get_acl
   v9fs_get_acl
   v9fs_inode_from_fid_dotl
   v9fs_get_tree

The allocation cannot succeed anyway and the failure is already mapped
to -EIO by __v9fs_get_acl(), so reject sizes above KMALLOC_MAX_SIZE
up front instead of tripping the allocator's warning. Larger ACLs that
kmalloc can still serve are unaffected.

Reported-by: syzbot+de6fd6789748a8aa64a0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=de6fd6789748a8aa64a0
Fixes: 85ff872d3f4a ("fs/9p: Implement POSIX ACL permission checking function")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
Hi Christian,

Thanks, agreed: XATTR_SIZE_MAX would wrongly limit servers with larger
9p xattrs. v2 caps at KMALLOC_MAX_SIZE, which is already the effective
limit, so nothing that worked before changes; it only avoids the
allocator warning for sizes that could never be served.

v2:
 - Bound by KMALLOC_MAX_SIZE instead of XATTR_SIZE_MAX so that ACLs from
   servers with larger 9p xattrs keep working (Christian).
 - Reworded the commit message accordingly.

v1: https://lore.kernel.org/all/20260919102958.142460-1-ngocthang2710.1999@gmail.com/

 fs/9p/acl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 0dd7e72bbdd3..915877cbaded 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -29,8 +29,8 @@ static struct posix_acl *v9fs_fid_get_acl(struct p9_fid *fid, const char *name)
 		return ERR_PTR(size);
 	if (size == 0)
 		return ERR_PTR(-ENODATA);
-	/* the size is server-controlled; a valid ACL fits in an xattr */
-	if (size > XATTR_SIZE_MAX)
+	/* the size is server-controlled; it cannot exceed what kmalloc serves */
+	if (size > KMALLOC_MAX_SIZE)
 		return ERR_PTR(-E2BIG);
 
 	value = kzalloc(size, GFP_NOFS);
-- 
2.43.0