[syzbot] [v9fs?] WARNING in __alloc_frozen_pages_noprof

[syzbot] [v9fs?] WARNING in __alloc_frozen_pages_noprof

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    af2ea8ab7a54 Add linux-next specific files for 20241205
git tree:       linux-next
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1369fde8580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=76f158395f6f15fd
dashboard link: https://syzkaller.appspot.com/bug?extid=03fb58296859d8dbab4d
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=15070820580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16e870f8580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/8af0861258fa/disk-af2ea8ab.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/ffb38cf7a344/vmlinux-af2ea8ab.xz
kernel image: https://storage.googleapis.com/syzbot-assets/6fbd2e50358a/bzImage-af2ea8ab.xz

The issue was bisected to:

commit 3a34b13a88caeb2800ab44a4918f230041b37dd9
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Fri Jul 30 22:42:34 2021 +0000

    pipe: make pipe writes always wake up readers

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=10759c0f980000
final oops:     https://syzkaller.appspot.com/x/report.txt?x=12759c0f980000
console output: https://syzkaller.appspot.com/x/log.txt?x=14759c0f980000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+03fb58296859d8dbab4d@syzkaller.appspotmail.com
Fixes: 3a34b13a88ca ("pipe: make pipe writes always wake up readers")

------------[ cut here ]------------
WARNING: CPU: 0 PID: 5830 at mm/page_alloc.c:4728 __alloc_frozen_pages_noprof+0x3c5/0x710 mm/page_alloc.c:4728
Modules linked in:
CPU: 0 UID: 0 PID: 5830 Comm: syz-executor405 Not tainted 6.13.0-rc1-next-20241205-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
RIP: 0010:__alloc_frozen_pages_noprof+0x3c5/0x710 mm/page_alloc.c:4728
Code: ff df 0f 85 09 01 00 00 44 89 e9 81 e1 7f ff ff ff a9 00 00 04 00 41 0f 44 cd 41 89 cd e9 f9 00 00 00 c6 05 87 3a 0c 0e 01 90 <0f> 0b 90 41 83 fc 0a 0f 86 13 fd ff ff 45 31 e4 48 c7 44 24 20 0e
RSP: 0018:ffffc90003e8f940 EFLAGS: 00010246
RAX: 0000000000000000 RBX: dffffc0000000000 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffc90003e8f9c8
RBP: ffffc90003e8fa50 R08: ffffc90003e8f9c7 R09: 0000000000000000
R10: ffffc90003e8f9a0 R11: fffff520007d1f39 R12: 0000000000000020
R13: 0000000000040d40 R14: 1ffff920007d1f30 R15: 1ffff920007d1f2c
FS:  0000555587715480(0000) GS:ffff8880b8600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020001000 CR3: 000000003352e000 CR4: 00000000003526f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 __alloc_pages_noprof+0xa/0x30 mm/page_alloc.c:4786
 __alloc_pages_node_noprof include/linux/gfp.h:269 [inline]
 alloc_pages_node_noprof include/linux/gfp.h:296 [inline]
 ___kmalloc_large_node+0x8b/0x1d0 mm/slub.c:4228
 __kmalloc_large_node_noprof+0x1a/0x80 mm/slub.c:4255
 __do_kmalloc_node mm/slub.c:4271 [inline]
 __kmalloc_noprof+0x339/0x4c0 mm/slub.c:4295
 kmalloc_noprof include/linux/slab.h:905 [inline]
 kzalloc_noprof include/linux/slab.h:1037 [inline]
 v9fs_fid_get_acl+0x4f/0x100 fs/9p/acl.c:32
 __v9fs_get_acl fs/9p/acl.c:66 [inline]
 v9fs_get_acl+0x96/0x350 fs/9p/acl.c:92
 v9fs_qid_iget_dotl fs/9p/vfs_inode_dotl.c:131 [inline]
 v9fs_inode_from_fid_dotl+0x22d/0x2c0 fs/9p/vfs_inode_dotl.c:154
 v9fs_get_new_inode_from_fid fs/9p/v9fs.h:251 [inline]
 v9fs_mount+0x718/0xa90 fs/9p/vfs_super.c:142
 legacy_get_tree+0xee/0x190 fs/fs_context.c:662
 vfs_get_tree+0x90/0x2b0 fs/super.c:1814
 do_new_mount+0x2be/0xb40 fs/namespace.c:3507
 do_mount fs/namespace.c:3847 [inline]
 __do_sys_mount fs/namespace.c:4057 [inline]
 __se_sys_mount+0x2d6/0x3c0 fs/namespace.c:4034
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f05fcd17de9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 f1 17 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffc85e9b418 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f05fcd17de9
RDX: 0000000020000b80 RSI: 00000000200003c0 RDI: 0000000000000000
RBP: 000000000000ec55 R08: 0000000020000580 R09: 00007ffc85e9b450
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffc85e9b450
R13: 00007ffc85e9b43c R14: 431bde82d7b634db R15: 00007f05fcd60087
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: WARNING in __alloc_frozen_pages_noprof

[Leo Stone]

syzbot creates a pipe and writes some data to it. It then creates a v9fs
mount using the pipe as transport. The data in the pipe specifies an ACL
of size 9 TB (9895604649984 bytes) for the root inode, causing kmalloc
to fail.

KMALLOC_MAX_SIZE is probably too loose of an upper bound for the size of
an ACL, but I didn't see an existing limit for V9FS like in e.g. NFS:

include/linux/nfsacl.h:
>/* Maximum number of ACL entries over NFS */
>#define NFS_ACL_MAX_ENTRIES     1024
>
>#define NFSACL_MAXWORDS         (2*(2+3*NFS_ACL_MAX_ENTRIES))
>#define NFSACL_MAXPAGES         ((2*(8+12*NFS_ACL_MAX_ENTRIES) + PAGE_SIZE-1) \
>                                 >> PAGE_SHIFT)
>        
>#define NFS_ACL_MAX_ENTRIES_INLINE      (5)
>#define NFS_ACL_INLINE_BUFSIZE  ((2*(2+3*NFS_ACL_MAX_ENTRIES_INLINE)) << 2)

#syz test

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

diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index eed551d8555f..1b9681d58f8d 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -28,6 +28,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);
+	if (size > KMALLOC_MAX_SIZE)
+		return ERR_PTR(-ERANGE);
 
 	value = kzalloc(size, GFP_NOFS);
 	if (!value)
-- 
2.43.0

Re: [syzbot] [v9fs?] WARNING in __alloc_frozen_pages_noprof

[syzbot]

Hello,

syzbot tried to test the proposed patch but the build/boot failed:

drivers/gpu/drm/i915/gt/intel_rc6.c:139:19: error: static assertion expression is not an integral constant expression
drivers/gpu/drm/i915/gt/intel_rc6.c:140:12: error: static assertion expression is not an integral constant expression
fs/bcachefs/str_hash.c:164:2: error: expected expression
fs/bcachefs/str_hash.c:165:30: error: use of undeclared identifier 'inode'
fs/bcachefs/str_hash.c:169:55: error: use of undeclared identifier 'inode'
fs/bcachefs/str_hash.c:171:40: error: use of undeclared identifier 'inode'


Tested on:

commit:         91e71d60 Add linux-next specific files for 20241211
git tree:       linux-next
kernel config:  https://syzkaller.appspot.com/x/.config?x=76f158395f6f15fd
dashboard link: https://syzkaller.appspot.com/bug?extid=03fb58296859d8dbab4d
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=12987544580000

Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[asmadeus]

Leo Stone wrote on Wed, Dec 11, 2024 at 12:02:40PM -0800:
> syzbot creates a pipe and writes some data to it. It then creates a v9fs
> mount using the pipe as transport. The data in the pipe specifies an ACL
> of size 9 TB (9895604649984 bytes) for the root inode, causing kmalloc
> to fail.

grmbl.

Sorry about that, there's been some paches ages ago to either cap xattrs
allocations to XATTR_SIZE_MAX, KMALLOC_MAX_SIZE, look into
vfs_getxattr_alloc or just flag the alloc __GFP_NOWARN:
https://lore.kernel.org/all/20240304-xattr_maxsize-v1-1-322357ec6bdf@codewreck.org/T/#u

and it was left forgotten because no decision was taken on something I
don't have time to think about

I've re-added everyone involved in Ccs, let's pick one and be done with
it.

Christian Schoenebeck's suggestion was something like this -- I guess
that's good enough for now and won't break anything (e.g. ACLs bigger
than XATTR_SIZE_MAX), so shall we go with that instead?

I don't care but let's get something in this cycle, the first patch is
almost one year old and this is ridiculous...

diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 8604e3377ee7..97f60b73bf16 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -37,8 +37,8 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
 	if (attr_size > buffer_size) {
 		if (buffer_size)
 			retval = -ERANGE;
-		else if (attr_size > SSIZE_MAX)
-			retval = -EOVERFLOW;
+		else if (attr_size > KMALLOC_MAX_SIZE)
+			retval = -E2BIG;
 		else /* request to get the attr_size */
 			retval = attr_size;
 	} else {

--
Dominique,
sleepy

Re: Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[Linus Torvalds]

On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> wrote:
>
> Christian Schoenebeck's suggestion was something like this -- I guess
> that's good enough for now and won't break anything (e.g. ACLs bigger
> than XATTR_SIZE_MAX), so shall we go with that instead?

Please use XATTR_SIZE_MAX. The KMALLOC_MAX_SIZE limit seems to make no
sense in this context.

Afaik the VFS layer doesn't allow getting an xattr bigger than
XATTR_SIZE_MAX anyway, and would return E2BIG for them later
regardless, so returning anything bigger wouldn't work anyway, even if
p9 tried to return such a thing up to some bigger limit.

No?

           Linus

Re: Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[Al Viro]

On Wed, Dec 11, 2024 at 01:32:26PM -0800, Linus Torvalds wrote:
> On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> wrote:
> >
> > Christian Schoenebeck's suggestion was something like this -- I guess
> > that's good enough for now and won't break anything (e.g. ACLs bigger
> > than XATTR_SIZE_MAX), so shall we go with that instead?
> 
> Please use XATTR_SIZE_MAX. The KMALLOC_MAX_SIZE limit seems to make no
> sense in this context.
> 
> Afaik the VFS layer doesn't allow getting an xattr bigger than
> XATTR_SIZE_MAX anyway, and would return E2BIG for them later
> regardless, so returning anything bigger wouldn't work anyway, even if
> p9 tried to return such a thing up to some bigger limit.

E2BIG on attempt to set, quiet cap to XATTR_SIZE_MAX on attempt to get
(i.e. never asking more than that from fs) and if filesystem complains
about XATTR_SIZE_MAX not being enough, E2BIG it is (instead of ERANGE
normally expected on "your buffer is too small for that").

Re: Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[Christian Schoenebeck]

On Wednesday, December 11, 2024 11:55:00 PM CET Al Viro wrote:
> On Wed, Dec 11, 2024 at 01:32:26PM -0800, Linus Torvalds wrote:
> > On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> wrote:
> > >
> > > Christian Schoenebeck's suggestion was something like this -- I guess
> > > that's good enough for now and won't break anything (e.g. ACLs bigger
> > > than XATTR_SIZE_MAX), so shall we go with that instead?
> > 
> > Please use XATTR_SIZE_MAX. The KMALLOC_MAX_SIZE limit seems to make no
> > sense in this context.
> > 
> > Afaik the VFS layer doesn't allow getting an xattr bigger than
> > XATTR_SIZE_MAX anyway, and would return E2BIG for them later
> > regardless, so returning anything bigger wouldn't work anyway, even if
> > p9 tried to return such a thing up to some bigger limit.
> 
> E2BIG on attempt to set, quiet cap to XATTR_SIZE_MAX on attempt to get
> (i.e. never asking more than that from fs) and if filesystem complains
> about XATTR_SIZE_MAX not being enough, E2BIG it is (instead of ERANGE
> normally expected on "your buffer is too small for that").

So that cap is effective even if that xattr does not go out to user space?

I mean the concern I had was about ACLs on guest, which are often mapped with 
9p to xattr on host and can become pretty big. So these were xattr not 
directly exposed to guest's user space.

/Christian

Re: Alloc cap limit for 9p xattrs (Was: WARNING in __alloc_frozen_pages_noprof)

[Christian Schoenebeck]

On Thursday, December 12, 2024 11:17:06 AM CET Christian Schoenebeck wrote:
> On Wednesday, December 11, 2024 11:55:00 PM CET Al Viro wrote:
> > On Wed, Dec 11, 2024 at 01:32:26PM -0800, Linus Torvalds wrote:
> > > On Wed, 11 Dec 2024 at 13:04, <asmadeus@codewreck.org> wrote:
> > > >
> > > > Christian Schoenebeck's suggestion was something like this -- I guess
> > > > that's good enough for now and won't break anything (e.g. ACLs bigger
> > > > than XATTR_SIZE_MAX), so shall we go with that instead?
> > > 
> > > Please use XATTR_SIZE_MAX. The KMALLOC_MAX_SIZE limit seems to make no
> > > sense in this context.
> > > 
> > > Afaik the VFS layer doesn't allow getting an xattr bigger than
> > > XATTR_SIZE_MAX anyway, and would return E2BIG for them later
> > > regardless, so returning anything bigger wouldn't work anyway, even if
> > > p9 tried to return such a thing up to some bigger limit.
> > 
> > E2BIG on attempt to set, quiet cap to XATTR_SIZE_MAX on attempt to get
> > (i.e. never asking more than that from fs) and if filesystem complains
> > about XATTR_SIZE_MAX not being enough, E2BIG it is (instead of ERANGE
> > normally expected on "your buffer is too small for that").
> 
> So that cap is effective even if that xattr does not go out to user space?
> 
> I mean the concern I had was about ACLs on guest, which are often mapped with 
> 9p to xattr on host and can become pretty big. So these were xattr not 
> directly exposed to guest's user space.

AFAICS it is not capped in this particular case: v9fs_fid_get_acl() calls
v9fs_fid_xattr_get() for getting the xattr, which in turn calls p9 client
functions to retrieve the xattr directly from 9p server (host). So the regular
Linux VFS layers are not involved here.

I also see no limit applied in fs/posix_acl.c when encoding/decoding ACLs.

And 9p server is not necessarily a Linux host, hence Linux's limit for xattr
do not necessarily apply.

So to me KMALLOC_MAX_SIZE (or better: 9p client's msize - header) still looks
right here, no?

/Christian

[PATCH] 9p: Limit xattr size to XATTR_SIZE_MAX

[Leo Stone]

syzbot triggered a warning in kmalloc by trying to mount a v9fs
filesystem from a pipe, after specifying an ACL size of 9TB for the
root inode in the data written to the pipe.

An xattr larger than XATTR_SIZE_MAX is considered invalid by the VFS
layer anyway. See do_getxattr():
>        } else if (error == -ERANGE && ctx->size >= XATTR_SIZE_MAX) {
>                /* The file system tried to returned a value bigger
>                   than XATTR_SIZE_MAX bytes. Not possible. */
>                error = -E2BIG;
>        }

Reported-by: syzbot+03fb58296859d8dbab4d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=03fb58296859d8dbab4d
Fixes: ebf46264a004 ("fs/9p: Add support user. xattr") 
Signed-off-by: Leo Stone <leocstone@gmail.com>
---
See: https://lore.kernel.org/all/675963eb.050a0220.17f54a.0038.GAE@google.com/T/ 
---
 fs/9p/xattr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/9p/xattr.c b/fs/9p/xattr.c
index 8604e3377ee7..97f60b73bf16 100644
--- a/fs/9p/xattr.c
+++ b/fs/9p/xattr.c
@@ -37,8 +37,8 @@ ssize_t v9fs_fid_xattr_get(struct p9_fid *fid, const char *name,
 	if (attr_size > buffer_size) {
 		if (buffer_size)
 			retval = -ERANGE;
-		else if (attr_size > SSIZE_MAX)
-			retval = -EOVERFLOW;
+		else if (attr_size > XATTR_SIZE_MAX)
+			retval = -E2BIG;
 		else /* request to get the attr_size */
 			retval = attr_size;
 	} else {
-- 
2.43.0