2024-03-04 14:08:48

by Sam Sun

[permalink] [raw]
Subject: [Bug] WARNING: zero-size vmalloc in sel_write_load

Dear developers and maintainers,

We encountered a warning in function sel_write_load(). It is tested on
kernel 6.8.0-rc7. Bug report is listed below.

```
WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
Modules linked in:
CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
<TASK>
__vmalloc_node mm/vmalloc.c:3385 [inline]
vmalloc+0x6b/0x80 mm/vmalloc.c:3418
sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
vfs_write+0x2a9/0xd80 fs/read_write.c:582
ksys_pwrite64 fs/read_write.c:699 [inline]
__do_sys_pwrite64 fs/read_write.c:709 [inline]
__se_sys_pwrite64 fs/read_write.c:706 [inline]
__x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7f40e7728f8d
Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
</TASK>
```

The cause of this bug is that in sel_write_load(), parameter "count"
is controlled by user, which could be zero. It is passed to vmalloc()
as an argument.

If you have any questions, please contact us.
Reported by: Yue Sun <[email protected]>
Reported by: xingwei lee <[email protected]>

Best Regards,
Yue


Attachments:
config (240.82 kB)
sel_write_load.c (1.02 kB)
Download all attachments

2024-03-04 19:19:46

by Paul Moore

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <[email protected]> wrote:
>
> Dear developers and maintainers,
>
> We encountered a warning in function sel_write_load(). It is tested on
> kernel 6.8.0-rc7. Bug report is listed below.
>
> ```
> WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> Modules linked in:
> CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> PKRU: 55555554
> Call Trace:
> <TASK>
> __vmalloc_node mm/vmalloc.c:3385 [inline]
> vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> vfs_write+0x2a9/0xd80 fs/read_write.c:582
> ksys_pwrite64 fs/read_write.c:699 [inline]
> __do_sys_pwrite64 fs/read_write.c:709 [inline]
> __se_sys_pwrite64 fs/read_write.c:706 [inline]
> __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x63/0x6b
> RIP: 0033:0x7f40e7728f8d
> Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> </TASK>
> ```
>
> The cause of this bug is that in sel_write_load(), parameter "count"
> is controlled by user, which could be zero. It is passed to vmalloc()
> as an argument.
>
> If you have any questions, please contact us.
> Reported by: Yue Sun <[email protected]>
> Reported by: xingwei lee <[email protected]>

Everything appears to be working as expected, vmalloc() caught the
zero-length allocation request, emitted the warning, returned NULL to
sel_write_load(), and sel_write_load() handled the error condition
triggered by vmalloc(0) returning NULL. Did you see any unexpected
behavior beyond this warning message above?

--
paul-moore.com

2024-03-04 20:11:41

by Christian Göttsche

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Mon, 4 Mar 2024 at 20:19, Paul Moore <[email protected]> wrote:
>
> On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <[email protected]> wrote:
> >
> > Dear developers and maintainers,
> >
> > We encountered a warning in function sel_write_load(). It is tested on
> > kernel 6.8.0-rc7. Bug report is listed below.
> >
> > ```
> > WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> > __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > Modules linked in:
> > CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> > 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> > 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> > RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> > RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> > RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> > R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> > FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > PKRU: 55555554
> > Call Trace:
> > <TASK>
> > __vmalloc_node mm/vmalloc.c:3385 [inline]
> > vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> > sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> > vfs_write+0x2a9/0xd80 fs/read_write.c:582
> > ksys_pwrite64 fs/read_write.c:699 [inline]
> > __do_sys_pwrite64 fs/read_write.c:709 [inline]
> > __se_sys_pwrite64 fs/read_write.c:706 [inline]
> > __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> > RIP: 0033:0x7f40e7728f8d
> > Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> > RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> > RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> > R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> > </TASK>
> > ```
> >
> > The cause of this bug is that in sel_write_load(), parameter "count"
> > is controlled by user, which could be zero. It is passed to vmalloc()
> > as an argument.
> >
> > If you have any questions, please contact us.
> > Reported by: Yue Sun <[email protected]>
> > Reported by: xingwei lee <[email protected]>
>
> Everything appears to be working as expected, vmalloc() caught the
> zero-length allocation request, emitted the warning, returned NULL to
> sel_write_load(), and sel_write_load() handled the error condition
> triggered by vmalloc(0) returning NULL. Did you see any unexpected
> behavior beyond this warning message above?

Probably because kernel warnings should not be reachable from
userspace - although in this case loading a policy is a highly
privileged operation - , because they mostly signal incorrect internal
state and can lead with the enabled option of panic_on_warn to system
halts.

I have two suggestions:

I. Can the documentation of vmalloc() mention that passing a size of 0
is discouraged?

II. Can the global SELinux state mutex in sel_write_load() be acquired
after the avc permission check, so that rouge processes with write
access to /load but not granted security { load_policy } can not pound
on that mutex?

>
> --
> paul-moore.com
>

2024-03-04 20:45:48

by Paul Moore

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Mon, Mar 4, 2024 at 3:11 PM Christian Göttsche
<[email protected]> wrote:
> On Mon, 4 Mar 2024 at 20:19, Paul Moore <[email protected]> wrote:
> > On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <[email protected]> wrote:
> > >
> > > Dear developers and maintainers,
> > >
> > > We encountered a warning in function sel_write_load(). It is tested on
> > > kernel 6.8.0-rc7. Bug report is listed below.
> > >
> > > ```
> > > WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> > > __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > Modules linked in:
> > > CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > > RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> > > 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> > > 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> > > RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> > > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> > > RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> > > RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> > > R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> > > R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> > > FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > PKRU: 55555554
> > > Call Trace:
> > > <TASK>
> > > __vmalloc_node mm/vmalloc.c:3385 [inline]
> > > vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> > > sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> > > vfs_write+0x2a9/0xd80 fs/read_write.c:582
> > > ksys_pwrite64 fs/read_write.c:699 [inline]
> > > __do_sys_pwrite64 fs/read_write.c:709 [inline]
> > > __se_sys_pwrite64 fs/read_write.c:706 [inline]
> > > __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> > > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> > > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> > > RIP: 0033:0x7f40e7728f8d
> > > Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> > > RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> > > RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> > > R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> > > </TASK>
> > > ```
> > >
> > > The cause of this bug is that in sel_write_load(), parameter "count"
> > > is controlled by user, which could be zero. It is passed to vmalloc()
> > > as an argument.
> > >
> > > If you have any questions, please contact us.
> > > Reported by: Yue Sun <[email protected]>
> > > Reported by: xingwei lee <[email protected]>
> >
> > Everything appears to be working as expected, vmalloc() caught the
> > zero-length allocation request, emitted the warning, returned NULL to
> > sel_write_load(), and sel_write_load() handled the error condition
> > triggered by vmalloc(0) returning NULL. Did you see any unexpected
> > behavior beyond this warning message above?
>
> Probably because kernel warnings should not be reachable from
> userspace ...

My question was asking if the reporter was seeing any unexpected
behavior *beyond* the warning message. I wanted to make sure they
weren't seeing anything else on their system that we should also take
into account.

> ... although in this case loading a policy is a highly
> privileged operation - , because they mostly signal incorrect internal
> state and can lead with the enabled option of panic_on_warn to system
> halts.
>
> I have two suggestions:
>
> I. Can the documentation of vmalloc() mention that passing a size of 0
> is discouraged?

One could always submit a patch and see what happens, that's almost
always the best way to get feedback.

> II. Can the global SELinux state mutex in sel_write_load() be acquired
> after the avc permission check, so that rouge processes with write
> access to /load but not granted security { load_policy } can not pound
> on that mutex?

We need to make sure that there is not a window between the
avc_has_perm() permission check and the actual policy load in
security_load_policy() where another task could race and cause some
unexpected behavior. For that reason I think we need to take the
mutex before the avc_has_perm() call, and we likely want to keep the
vmalloc()/copy_from_user() after the permission check for the same
reason you wanted to delay taking the mutex.

We probably could consider moving the @ppos and @count sanity checks
before the mutex.

--
paul-moore.com

2024-03-05 02:34:17

by Sam Sun

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Tue, Mar 5, 2024 at 4:45 AM Paul Moore <[email protected]> wrote:
>
> On Mon, Mar 4, 2024 at 3:11 PM Christian Göttsche
> <[email protected]> wrote:
> > On Mon, 4 Mar 2024 at 20:19, Paul Moore <[email protected]> wrote:
> > > On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <[email protected]> wrote:
> > > >
> > > > Dear developers and maintainers,
> > > >
> > > > We encountered a warning in function sel_write_load(). It is tested on
> > > > kernel 6.8.0-rc7. Bug report is listed below.
> > > >
> > > > ```
> > > > WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> > > > __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > Modules linked in:
> > > > CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > > > RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> > > > 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> > > > 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> > > > RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> > > > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> > > > RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> > > > RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> > > > R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> > > > R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> > > > FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > PKRU: 55555554
> > > > Call Trace:
> > > > <TASK>
> > > > __vmalloc_node mm/vmalloc.c:3385 [inline]
> > > > vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> > > > sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> > > > vfs_write+0x2a9/0xd80 fs/read_write.c:582
> > > > ksys_pwrite64 fs/read_write.c:699 [inline]
> > > > __do_sys_pwrite64 fs/read_write.c:709 [inline]
> > > > __se_sys_pwrite64 fs/read_write.c:706 [inline]
> > > > __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> > > > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> > > > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> > > > RIP: 0033:0x7f40e7728f8d
> > > > Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> > > > RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> > > > RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> > > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> > > > R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> > > > </TASK>
> > > > ```
> > > >
> > > > The cause of this bug is that in sel_write_load(), parameter "count"
> > > > is controlled by user, which could be zero. It is passed to vmalloc()
> > > > as an argument.
> > > >
> > > > If you have any questions, please contact us.
> > > > Reported by: Yue Sun <[email protected]>
> > > > Reported by: xingwei lee <[email protected]>
> > >
> > > Everything appears to be working as expected, vmalloc() caught the
> > > zero-length allocation request, emitted the warning, returned NULL to
> > > sel_write_load(), and sel_write_load() handled the error condition
> > > triggered by vmalloc(0) returning NULL. Did you see any unexpected
> > > behavior beyond this warning message above?
> >
> > Probably because kernel warnings should not be reachable from
> > userspace ...
>
> My question was asking if the reporter was seeing any unexpected
> behavior *beyond* the warning message. I wanted to make sure they
> weren't seeing anything else on their system that we should also take
> into account.
>

I didn't see any unexpected behavior beyond this warning message. You
are right, everything appears to be working as expected. Like
Christian said, I enabled kernel panic_on_warn config. I thought
kernel warning was something worthy to report, but I was wrong. In
future reports, I will check carefully to eliminate kernel warnings
like "vmalloc zero" and "page size alloc too large" if they don't have
unexpected behaviors. Sorry for wasting your time analyzing it, and
thanks for pointing out my mistake!

> > ... although in this case loading a policy is a highly
> > privileged operation - , because they mostly signal incorrect internal
> > state and can lead with the enabled option of panic_on_warn to system
> > halts.
> >
> > I have two suggestions:
> >
> > I. Can the documentation of vmalloc() mention that passing a size of 0
> > is discouraged?
>
> One could always submit a patch and see what happens, that's almost
> always the best way to get feedback.
>
> > II. Can the global SELinux state mutex in sel_write_load() be acquired
> > after the avc permission check, so that rouge processes with write
> > access to /load but not granted security { load_policy } can not pound
> > on that mutex?
>
> We need to make sure that there is not a window between the
> avc_has_perm() permission check and the actual policy load in
> security_load_policy() where another task could race and cause some
> unexpected behavior. For that reason I think we need to take the
> mutex before the avc_has_perm() call, and we likely want to keep the
> vmalloc()/copy_from_user() after the permission check for the same
> reason you wanted to delay taking the mutex.
>
> We probably could consider moving the @ppos and @count sanity checks
> before the mutex.
>
> --
> paul-moore.com

Best Regards,
Yue

2024-03-05 03:16:36

by Paul Moore

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Mon, Mar 4, 2024 at 9:32 PM Sam Sun <[email protected]> wrote:
> On Tue, Mar 5, 2024 at 4:45 AM Paul Moore <[email protected]> wrote:
> > On Mon, Mar 4, 2024 at 3:11 PM Christian Göttsche
> > <[email protected]> wrote:
> > > On Mon, 4 Mar 2024 at 20:19, Paul Moore <[email protected]> wrote:
> > > > On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <samsun1006219@gmailcom> wrote:
> > > > >
> > > > > Dear developers and maintainers,
> > > > >
> > > > > We encountered a warning in function sel_write_load(). It is tested on
> > > > > kernel 6.8.0-rc7. Bug report is listed below.
> > > > >
> > > > > ```
> > > > > WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> > > > > __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > > Modules linked in:
> > > > > CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> > > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > > > > RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > > Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> > > > > 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> > > > > 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> > > > > RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> > > > > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> > > > > RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> > > > > RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> > > > > R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> > > > > R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> > > > > FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> > > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > PKRU: 55555554
> > > > > Call Trace:
> > > > > <TASK>
> > > > > __vmalloc_node mm/vmalloc.c:3385 [inline]
> > > > > vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> > > > > sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> > > > > vfs_write+0x2a9/0xd80 fs/read_write.c:582
> > > > > ksys_pwrite64 fs/read_write.c:699 [inline]
> > > > > __do_sys_pwrite64 fs/read_write.c:709 [inline]
> > > > > __se_sys_pwrite64 fs/read_write.c:706 [inline]
> > > > > __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> > > > > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > > > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> > > > > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> > > > > RIP: 0033:0x7f40e7728f8d
> > > > > Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> > > > > RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> > > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> > > > > RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> > > > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> > > > > R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> > > > > </TASK>
> > > > > ```
> > > > >
> > > > > The cause of this bug is that in sel_write_load(), parameter "count"
> > > > > is controlled by user, which could be zero. It is passed to vmalloc()
> > > > > as an argument.
> > > > >
> > > > > If you have any questions, please contact us.
> > > > > Reported by: Yue Sun <[email protected]>
> > > > > Reported by: xingwei lee <[email protected]>
> > > >
> > > > Everything appears to be working as expected, vmalloc() caught the
> > > > zero-length allocation request, emitted the warning, returned NULL to
> > > > sel_write_load(), and sel_write_load() handled the error condition
> > > > triggered by vmalloc(0) returning NULL. Did you see any unexpected
> > > > behavior beyond this warning message above?
> > >
> > > Probably because kernel warnings should not be reachable from
> > > userspace ...
> >
> > My question was asking if the reporter was seeing any unexpected
> > behavior *beyond* the warning message. I wanted to make sure they
> > weren't seeing anything else on their system that we should also take
> > into account.
>
> I didn't see any unexpected behavior beyond this warning message. You
> are right, everything appears to be working as expected. Like
> Christian said, I enabled kernel panic_on_warn config. I thought
> kernel warning was something worthy to report, but I was wrong. In
> future reports, I will check carefully to eliminate kernel warnings
> like "vmalloc zero" and "page size alloc too large" if they don't have
> unexpected behaviors. Sorry for wasting your time analyzing it, and
> thanks for pointing out my mistake!

Thanks for the quick reply, I just wanted to make sure there wasn't
some other bug that was also triggered by passing a 0 count value; I'm
glad to hear there isn't. Regardless, there is likely some value in
protecting against 0 count/size values and I just posted a patch to do
just that (and some other misc cleanups to sel_write_load()).

--
paul-moore.com

2024-03-05 03:28:58

by Sam Sun

[permalink] [raw]
Subject: Re: [Bug] WARNING: zero-size vmalloc in sel_write_load

On Tue, Mar 5, 2024 at 11:16 AM Paul Moore <[email protected]> wrote:
>
> On Mon, Mar 4, 2024 at 9:32 PM Sam Sun <[email protected]> wrote:
> > On Tue, Mar 5, 2024 at 4:45 AM Paul Moore <[email protected]> wrote:
> > > On Mon, Mar 4, 2024 at 3:11 PM Christian Göttsche
> > > <[email protected]> wrote:
> > > > On Mon, 4 Mar 2024 at 20:19, Paul Moore <[email protected]> wrote:
> > > > > On Mon, Mar 4, 2024 at 9:08 AM Sam Sun <[email protected]> wrote:
> > > > > >
> > > > > > Dear developers and maintainers,
> > > > > >
> > > > > > We encountered a warning in function sel_write_load(). It is tested on
> > > > > > kernel 6.8.0-rc7. Bug report is listed below.
> > > > > >
> > > > > > ```
> > > > > > WARNING: CPU: 1 PID: 8109 at mm/vmalloc.c:3247
> > > > > > __vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > > > Modules linked in:
> > > > > > CPU: 1 PID: 8109 Comm: syz-executor370 Not tainted 6.7.0-rc7 #1
> > > > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> > > > > > RIP: 0010:__vmalloc_node_range+0x10a0/0x1490 mm/vmalloc.c:3247
> > > > > > Code: 65 48 2b 04 25 28 00 00 00 0f 85 98 02 00 00 48 81 c4 70 01 00
> > > > > > 00 4c 89 e0 5b 5d 41 5c 41 5d 41 5e 41 5f c3 e8 71 43 b7 ff 90 <0f> 0b
> > > > > > 90 45 31 e4 eb a1 e8 63 43 b7 ff 48 8b 4c 24 40 31 f6 45 31
> > > > > > RSP: 0018:ffffc90002adf9c0 EFLAGS: 00010293
> > > > > > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: ffffffff81cdc194
> > > > > > RDX: ffff888022124ec0 RSI: ffffffff81cdd16f RDI: 0000000000000007
> > > > > > RBP: 0000000000000000 R08: 0000000000000007 R09: 0000000000000000
> > > > > > R10: 0000000000000000 R11: 0000000000000001 R12: ffff888107373a48
> > > > > > R13: dffffc0000000000 R14: 0000000000000000 R15: ffffc90002adfec8
> > > > > > FS: 00005555560953c0(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> > > > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > > > > > CR2: 0000000020000010 CR3: 000000010503d000 CR4: 0000000000750ef0
> > > > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > > > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > > > > > PKRU: 55555554
> > > > > > Call Trace:
> > > > > > <TASK>
> > > > > > __vmalloc_node mm/vmalloc.c:3385 [inline]
> > > > > > vmalloc+0x6b/0x80 mm/vmalloc.c:3418
> > > > > > sel_write_load+0x27f/0x19d0 security/selinux/selinuxfs.c:603
> > > > > > vfs_write+0x2a9/0xd80 fs/read_write.c:582
> > > > > > ksys_pwrite64 fs/read_write.c:699 [inline]
> > > > > > __do_sys_pwrite64 fs/read_write.c:709 [inline]
> > > > > > __se_sys_pwrite64 fs/read_write.c:706 [inline]
> > > > > > __x64_sys_pwrite64+0x1f3/0x250 fs/read_write.c:706
> > > > > > do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> > > > > > do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
> > > > > > entry_SYSCALL_64_after_hwframe+0x63/0x6b
> > > > > > RIP: 0033:0x7f40e7728f8d
> > > > > > Code: 28 c3 e8 46 1e 00 00 66 0f 1f 44 00 00 f3 0f 1e fa 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:00007fff5bf39508 EFLAGS: 00000246 ORIG_RAX: 0000000000000012
> > > > > > RAX: ffffffffffffffda RBX: 00007fff5bf39708 RCX: 00007f40e7728f8d
> > > > > > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> > > > > > RBP: 0000000000000001 R08: 0000000000000000 R09: 00007fff5bf39708
> > > > > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> > > > > > R13: 00007fff5bf396f8 R14: 00007f40e77a6530 R15: 0000000000000001
> > > > > > </TASK>
> > > > > > ```
> > > > > >
> > > > > > The cause of this bug is that in sel_write_load(), parameter "count"
> > > > > > is controlled by user, which could be zero. It is passed to vmalloc()
> > > > > > as an argument.
> > > > > >
> > > > > > If you have any questions, please contact us.
> > > > > > Reported by: Yue Sun <[email protected]>
> > > > > > Reported by: xingwei lee <[email protected]>
> > > > >
> > > > > Everything appears to be working as expected, vmalloc() caught the
> > > > > zero-length allocation request, emitted the warning, returned NULL to
> > > > > sel_write_load(), and sel_write_load() handled the error condition
> > > > > triggered by vmalloc(0) returning NULL. Did you see any unexpected
> > > > > behavior beyond this warning message above?
> > > >
> > > > Probably because kernel warnings should not be reachable from
> > > > userspace ...
> > >
> > > My question was asking if the reporter was seeing any unexpected
> > > behavior *beyond* the warning message. I wanted to make sure they
> > > weren't seeing anything else on their system that we should also take
> > > into account.
> >
> > I didn't see any unexpected behavior beyond this warning message. You
> > are right, everything appears to be working as expected. Like
> > Christian said, I enabled kernel panic_on_warn config. I thought
> > kernel warning was something worthy to report, but I was wrong. In
> > future reports, I will check carefully to eliminate kernel warnings
> > like "vmalloc zero" and "page size alloc too large" if they don't have
> > unexpected behaviors. Sorry for wasting your time analyzing it, and
> > thanks for pointing out my mistake!
>
> Thanks for the quick reply, I just wanted to make sure there wasn't
> some other bug that was also triggered by passing a 0 count value; I'm
> glad to hear there isn't. Regardless, there is likely some value in
> protecting against 0 count/size values and I just posted a patch to do
> just that (and some other misc cleanups to sel_write_load()).
>
> --
> paul-moore.com

Maybe I should compile the kernel without panic_on_warn config, so
that I can see how zero count / size values are passed through code
and check if there is another potential threat. Thanks for your
advice!

Best Regards,
Yue