* Crash in VirtualBox virtual machines running kernel 6.5 @ 2023-07-19 15:02 Larry Finger 2023-07-19 15:31 ` Kees Cook 0 siblings, 1 reply; 7+ messages in thread From: Larry Finger @ 2023-07-19 15:02 UTC (permalink / raw) To: Hans de Goede, Azeem Shaikh, Kees Cook; +Cc: LKML Hi, When I try to start a VirtualBox virtual machine running kernel 6.5-rc2, it gets a kernel bug as follows while trying to mount a vboxsf-shared mount: Jul 19 08:48:19 localhost kernel: detected buffer overflow in strscpy Jul 19 08:48:19 localhost kernel: ------------[ cut here ]------------ Jul 19 08:48:19 localhost kernel: kernel BUG at lib/string_helpers.c:1031! Jul 19 08:48:19 localhost kernel: invalid opcode: 0000 [#1] PREEMPT SMP PTI Jul 19 08:48:19 localhost kernel: CPU: 2 PID: 507 Comm: mount.vboxsf Not tainted 6.5.0-rc2-1.gc159bc5-default #1 openSUSE Tumbleweed (unreleased) 4ec79f0435e04b515527e428ee3d62093b7f6e02 Jul 19 08:48:19 localhost kernel: Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006 Jul 19 08:48:19 localhost kernel: RIP: 0010:fortify_panic+0x13/0x20 Jul 19 08:48:19 localhost kernel: Code: 41 5d c3 cc cc cc cc 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 48 89 fe 48 c7 c7 68 e5 c6 93 e8 ed a7 b2 ff <0f> 0b 66 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 Jul 19 08:48:19 localhost kernel: RSP: 0018:ffffa72a4047fd40 EFLAGS: 00010246 Jul 19 08:48:19 localhost kernel: RAX: 0000000000000023 RBX: ffff903ac9d8ac00 RCX: 0000000000000000 Jul 19 08:48:19 localhost kernel: RDX: 0000000000000000 RSI: ffff903adbd274c0 RDI: ffff903adbd274c0 Jul 19 08:48:19 localhost kernel: RBP: 0000000000000002 R08: 0000000000000000 R09: ffffa72a4047fc00 Jul 19 08:48:19 localhost kernel: R10: 0000000000000003 R11: ffffffff94558568 R12: ffff903ac2013f00 Jul 19 08:48:19 localhost kernel: R13: ffff903ac8492800 R14: ffff903ac7b9dd20 R15: 0000000000000007 Jul 19 08:48:19 localhost kernel: FS: 00007f22f0cec740(0000) GS:ffff903adbd00000(0000) knlGS:0000000000000000 Jul 19 08:48:19 localhost kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 Jul 19 08:48:19 localhost kernel: CR2: 00007f22f0e7f574 CR3: 0000000109474001 CR4: 00000000000706e0 Jul 19 08:48:19 localhost kernel: Call Trace: Jul 19 08:48:19 localhost kernel: <TASK> Jul 19 08:48:19 localhost kernel: ? die+0x36/0x90 Jul 19 08:48:19 localhost kernel: ? do_trap+0xda/0x100 Jul 19 08:48:19 localhost kernel: ? fortify_panic+0x13/0x20 Jul 19 08:48:19 localhost kernel: ? do_error_trap+0x6a/0x90 Jul 19 08:48:19 localhost kernel: ? fortify_panic+0x13/0x20 Jul 19 08:48:19 localhost kernel: ? exc_invalid_op+0x50/0x70 Jul 19 08:48:19 localhost kernel: ? fortify_panic+0x13/0x20 Jul 19 08:48:19 localhost kernel: ? asm_exc_invalid_op+0x1a/0x20 Jul 19 08:48:19 localhost kernel: ? fortify_panic+0x13/0x20 Jul 19 08:48:19 localhost kernel: vboxsf_fill_super+0x3bc/0x3c0 [vboxsf 447dff7257fbc53f0b47ed873d2b02eb4773401c] Jul 19 08:48:19 localhost kernel: ? __pfx_vboxsf_fill_super+0x10/0x10 [vboxsf 447dff7257fbc53f0b47ed873d2b02eb4773401c] Jul 19 08:48:19 localhost kernel: vfs_get_super+0x6e/0xe0 Jul 19 08:48:19 localhost kernel: vfs_get_tree+0x29/0xd0 Jul 19 08:48:19 localhost kernel: path_mount+0x491/0xac0 Jul 19 08:48:19 localhost kernel: __x64_sys_mount+0x109/0x140 Jul 19 08:48:19 localhost kernel: do_syscall_64+0x60/0x90 Jul 19 08:48:19 localhost kernel: ? count_memcg_events.constprop.0+0x1a/0x30 Jul 19 08:48:19 localhost kernel: ? handle_mm_fault+0x9e/0x350 Jul 19 08:48:19 localhost kernel: ? do_user_addr_fault+0x225/0x640 Jul 19 08:48:19 localhost kernel: ? exc_page_fault+0x71/0x160 Jul 19 08:48:19 localhost kernel: entry_SYSCALL_64_after_hwframe+0x6e/0xd8 Jul 19 08:48:19 localhost kernel: RIP: 0033:0x7f22f0e05b9e Jul 19 08:48:19 localhost kernel: Code: c3 66 2e 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f 1e fa 66 90 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 3a 02 0d 00 f7 d8 64 89 01 48 Jul 19 08:48:19 localhost kernel: RSP: 002b:00007ffe8ba4bab8 EFLAGS: 00000212 ORIG_RAX: 00000000000000a5 Jul 19 08:48:19 localhost kernel: RAX: ffffffffffffffda RBX: 00007ffe8ba4df04 RCX: 00007f22f0e05b9e Jul 19 08:48:19 localhost kernel: RDX: 000055e9ab199ba8 RSI: 00007ffe8ba4df0b RDI: 00007ffe8ba4df04 Jul 19 08:48:19 localhost kernel: RBP: 00007ffe8ba4bc00 R08: 00007ffe8ba4bf50 R09: 0000000000000000 Jul 19 08:48:19 localhost kernel: R10: 0000000000000004 R11: 0000000000000212 R12: 00007ffe8ba4df0b Jul 19 08:48:19 localhost kernel: R13: 0000000000000011 R14: 00007ffe8ba4bf50 R15: 00007ffe8ba4df29 Jul 19 08:48:19 localhost kernel: </TASK> Jul 19 08:48:19 localhost kernel: Modules linked in: vboxsf vboxguest fuse efi_pstore configfs dmi_sysfs ip_tables x_tables crct10dif_pclmul crc32_pclmul polyval_clmulni polyval_generic sr_mod ohci_pci gf128mul cdrom ohci_hcd ehci_pci ehci_hcd ghash_clmulni_intel sha512_ssse3 ata_generic vmwgfx video usbcore drm_ttm_helper ttm aesni_intel crypto_simd cryptd ata_piix wmi serio_raw btrfs blake2b_generic libcrc32c crc32c_intel xor raid6_pq sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua msr efivarfs Jul 19 08:48:19 localhost kernel: ---[ end trace 0000000000000000 ]--- The traceback points to the strscpy() added in commit 883f8fe87686d, which ironically was submitted to avoid buffer overflows using strlcpy(); however, I do not think that is the problem. My suspicion is that it comes from struct shfl_string, and the definition of the variable-length arrays in the union, and that their lengths are confusing the kernel's string handling routines. I will be happy to test any proposed patches. Larry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 15:02 Crash in VirtualBox virtual machines running kernel 6.5 Larry Finger @ 2023-07-19 15:31 ` Kees Cook 2023-07-19 15:34 ` Kees Cook 2023-07-19 15:36 ` Kees Cook 0 siblings, 2 replies; 7+ messages in thread From: Kees Cook @ 2023-07-19 15:31 UTC (permalink / raw) To: Larry Finger Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva On Wed, Jul 19, 2023 at 10:02:36AM -0500, Larry Finger wrote: > Hi, > > When I try to start a VirtualBox virtual machine running kernel 6.5-rc2, it > gets a kernel bug as follows while trying to mount a vboxsf-shared mount: > > Jul 19 08:48:19 localhost kernel: detected buffer overflow in strscpy > Jul 19 08:48:19 localhost kernel: ------------[ cut here ]------------ > Jul 19 08:48:19 localhost kernel: kernel BUG at lib/string_helpers.c:1031! > [...] > Jul 19 08:48:19 localhost kernel: Call Trace: > [...] > Jul 19 08:48:19 localhost kernel: vboxsf_fill_super+0x3bc/0x3c0 [vboxsf 447dff7257fbc53f0b47ed873d2b02eb4773401c] > [...] > > The traceback points to the strscpy() added in commit 883f8fe87686d, which > ironically was submitted to avoid buffer overflows using strlcpy(); however, > I do not think that is the problem. My suspicion is that it comes from > struct shfl_string, and the definition of the variable-length arrays in the > union, and that their lengths are confusing the kernel's string handling > routines. Ah, hm, I think this may still warn with 883f8fe87686d reverted, as it seems the issue is the fake flexible arrays in struct shfl_string. Likely the patch manifesting the false positive is df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3"), if you're building with GCC 13. > I will be happy to test any proposed patches. Thank! Can you see if this fixes it? diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h index aca829062c12..243d1b91bb45 100644 --- a/fs/vboxsf/shfl_hostintf.h +++ b/fs/vboxsf/shfl_hostintf.h @@ -68,9 +68,8 @@ struct shfl_string { /** UTF-8 or UTF-16 string. Nul terminated. */ union { - u8 utf8[2]; - u16 utf16[1]; - u16 ucs2[1]; /* misnomer, use utf16. */ + DECLARE_FLEX_ARRAY(u8, utf8); + DECLARE_FLEX_ARRAY(u16, utf16); } string; }; VMMDEV_ASSERT_SIZE(shfl_string, 6); (I note that "ucs" is used in the kernel source, and contains a comment that it shouldn't be used, so I removed it.) -- Kees Cook ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 15:31 ` Kees Cook @ 2023-07-19 15:34 ` Kees Cook 2023-07-19 15:36 ` Kees Cook 1 sibling, 0 replies; 7+ messages in thread From: Kees Cook @ 2023-07-19 15:34 UTC (permalink / raw) To: Larry Finger Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva On Wed, Jul 19, 2023 at 08:31:08AM -0700, Kees Cook wrote: > diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h > index aca829062c12..243d1b91bb45 100644 > --- a/fs/vboxsf/shfl_hostintf.h > +++ b/fs/vboxsf/shfl_hostintf.h > @@ -68,9 +68,8 @@ struct shfl_string { > > /** UTF-8 or UTF-16 string. Nul terminated. */ > union { > - u8 utf8[2]; > - u16 utf16[1]; > - u16 ucs2[1]; /* misnomer, use utf16. */ > + DECLARE_FLEX_ARRAY(u8, utf8); > + DECLARE_FLEX_ARRAY(u16, utf16); > } string; > }; > VMMDEV_ASSERT_SIZE(shfl_string, 6); Oops, this doesn't even compile. Let me get that fixed... -- Kees Cook ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 15:31 ` Kees Cook 2023-07-19 15:34 ` Kees Cook @ 2023-07-19 15:36 ` Kees Cook 2023-07-19 21:24 ` Larry Finger 1 sibling, 1 reply; 7+ messages in thread From: Kees Cook @ 2023-07-19 15:36 UTC (permalink / raw) To: Larry Finger Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva Okay, please try: diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h index aca829062c12..902fe3224453 100644 --- a/fs/vboxsf/shfl_hostintf.h +++ b/fs/vboxsf/shfl_hostintf.h @@ -68,12 +68,11 @@ struct shfl_string { /** UTF-8 or UTF-16 string. Nul terminated. */ union { - u8 utf8[2]; - u16 utf16[1]; - u16 ucs2[1]; /* misnomer, use utf16. */ + DECLARE_FLEX_ARRAY(u8, utf8); + DECLARE_FLEX_ARRAY(u16, utf16); } string; }; -VMMDEV_ASSERT_SIZE(shfl_string, 6); +VMMDEV_ASSERT_SIZE(shfl_string, 4); /* The size of shfl_string w/o the string part. */ #define SHFLSTRING_HEADER_SIZE 4 The size assert doesn't seem to be used anywhere else, but I can do a more careful binary analysis later today... -- Kees Cook ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 15:36 ` Kees Cook @ 2023-07-19 21:24 ` Larry Finger 2023-07-19 22:37 ` Kees Cook 0 siblings, 1 reply; 7+ messages in thread From: Larry Finger @ 2023-07-19 21:24 UTC (permalink / raw) To: Kees Cook Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva On 7/19/23 10:36, Kees Cook wrote: > Okay, please try: > > diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h > index aca829062c12..902fe3224453 100644 > --- a/fs/vboxsf/shfl_hostintf.h > +++ b/fs/vboxsf/shfl_hostintf.h > @@ -68,12 +68,11 @@ struct shfl_string { > > /** UTF-8 or UTF-16 string. Nul terminated. */ > union { > - u8 utf8[2]; > - u16 utf16[1]; > - u16 ucs2[1]; /* misnomer, use utf16. */ > + DECLARE_FLEX_ARRAY(u8, utf8); > + DECLARE_FLEX_ARRAY(u16, utf16); > } string; > }; > -VMMDEV_ASSERT_SIZE(shfl_string, 6); > +VMMDEV_ASSERT_SIZE(shfl_string, 4); > > /* The size of shfl_string w/o the string part. */ > #define SHFLSTRING_HEADER_SIZE 4 > > > The size assert doesn't seem to be used anywhere else, but I can do a > more careful binary analysis later today...'' Kees, The testing was harder than I expected. My standard kernel would not load the system disk on the VM, thus I had to build one using my distros configuration. It is really painful to wait for all those drivers to build, but I figured that might be faster than trying to find the incorrect parameter. I can finally report that vboxsf no longer generated a BUG. It is getting farther, but I am not done yet. It now generates a line that says "Unknown parameter tag" and then hangs. I have not tracked that down yet. I was not aware of the DECLARE_FLEX_ARRAY() macro. I had considered removing the union and creating a simple string[] declaration, but I do not mess with file systems, and decided to let the experts handle it. I will let you know what I find about that unknown parameter. It probably is coming from VirtualBox. Thanks, Larry ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 21:24 ` Larry Finger @ 2023-07-19 22:37 ` Kees Cook 2023-07-20 14:51 ` Larry Finger 0 siblings, 1 reply; 7+ messages in thread From: Kees Cook @ 2023-07-19 22:37 UTC (permalink / raw) To: Larry Finger, Kees Cook Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva On July 19, 2023 2:24:52 PM PDT, Larry Finger <Larry.Finger@lwfinger.net> wrote: >On 7/19/23 10:36, Kees Cook wrote: >> Okay, please try: >> >> diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h >> index aca829062c12..902fe3224453 100644 >> --- a/fs/vboxsf/shfl_hostintf.h >> +++ b/fs/vboxsf/shfl_hostintf.h >> @@ -68,12 +68,11 @@ struct shfl_string { >> /** UTF-8 or UTF-16 string. Nul terminated. */ >> union { >> - u8 utf8[2]; >> - u16 utf16[1]; >> - u16 ucs2[1]; /* misnomer, use utf16. */ >> + DECLARE_FLEX_ARRAY(u8, utf8); >> + DECLARE_FLEX_ARRAY(u16, utf16); >> } string; >> }; >> -VMMDEV_ASSERT_SIZE(shfl_string, 6); >> +VMMDEV_ASSERT_SIZE(shfl_string, 4); >> /* The size of shfl_string w/o the string part. */ >> #define SHFLSTRING_HEADER_SIZE 4 >> >> >> The size assert doesn't seem to be used anywhere else, but I can do a >> more careful binary analysis later today...'' > >Kees, > >The testing was harder than I expected. My standard kernel would not load the system disk on the VM, thus I had to build one using my distros configuration. It is really painful to wait for all those drivers to build, but I figured that might be faster than trying to find the incorrect parameter. > >I can finally report that vboxsf no longer generated a BUG. It is getting farther, but I am not done yet. It now generates a line that says "Unknown parameter tag" and then hangs. I have not tracked that down yet. > >I was not aware of the DECLARE_FLEX_ARRAY() macro. I had considered removing the union and creating a simple string[] declaration, but I do not mess with file systems, and decided to let the experts handle it. > >I will let you know what I find about that unknown parameter. It probably is coming from VirtualBox. It's possible the size really needs to stay 6 bytes. In that case, try adding a "u8 legacy_padding[2]" to the union and restore the VMMDEV_ASSERT_SIZE? -- Kees Cook ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Crash in VirtualBox virtual machines running kernel 6.5 2023-07-19 22:37 ` Kees Cook @ 2023-07-20 14:51 ` Larry Finger 0 siblings, 0 replies; 7+ messages in thread From: Larry Finger @ 2023-07-20 14:51 UTC (permalink / raw) To: Kees Cook, Kees Cook Cc: Hans de Goede, Azeem Shaikh, LKML, linux-hardening, Gustavo A. R. Silva On 7/19/23 17:37, Kees Cook wrote: > On July 19, 2023 2:24:52 PM PDT, Larry Finger <Larry.Finger@lwfinger.net> wrote: >> On 7/19/23 10:36, Kees Cook wrote: >>> Okay, please try: >>> >>> diff --git a/fs/vboxsf/shfl_hostintf.h b/fs/vboxsf/shfl_hostintf.h >>> index aca829062c12..902fe3224453 100644 >>> --- a/fs/vboxsf/shfl_hostintf.h >>> +++ b/fs/vboxsf/shfl_hostintf.h >>> @@ -68,12 +68,11 @@ struct shfl_string { >>> /** UTF-8 or UTF-16 string. Nul terminated. */ >>> union { >>> - u8 utf8[2]; >>> - u16 utf16[1]; >>> - u16 ucs2[1]; /* misnomer, use utf16. */ >>> + DECLARE_FLEX_ARRAY(u8, utf8); >>> + DECLARE_FLEX_ARRAY(u16, utf16); >>> } string; >>> }; >>> -VMMDEV_ASSERT_SIZE(shfl_string, 6); >>> +VMMDEV_ASSERT_SIZE(shfl_string, 4); >>> /* The size of shfl_string w/o the string part. */ >>> #define SHFLSTRING_HEADER_SIZE 4 >>> >>> >>> The size assert doesn't seem to be used anywhere else, but I can do a >>> more careful binary analysis later today...'' >> >> Kees, >> >> The testing was harder than I expected. My standard kernel would not load the system disk on the VM, thus I had to build one using my distros configuration. It is really painful to wait for all those drivers to build, but I figured that might be faster than trying to find the incorrect parameter. >> >> I can finally report that vboxsf no longer generated a BUG. It is getting farther, but I am not done yet. It now generates a line that says "Unknown parameter tag" and then hangs. I have not tracked that down yet. >> >> I was not aware of the DECLARE_FLEX_ARRAY() macro. I had considered removing the union and creating a simple string[] declaration, but I do not mess with file systems, and decided to let the experts handle it. >> >> I will let you know what I find about that unknown parameter. It probably is coming from VirtualBox. > > It's possible the size really needs to stay 6 bytes. In that case, try adding a "u8 legacy_padding[2]" to the union and restore the VMMDEV_ASSERT_SIZE? Kees, You win the prize. Adding the 2 bytes of padding restored operations. Thanks, Larry ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-20 14:51 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-19 15:02 Crash in VirtualBox virtual machines running kernel 6.5 Larry Finger 2023-07-19 15:31 ` Kees Cook 2023-07-19 15:34 ` Kees Cook 2023-07-19 15:36 ` Kees Cook 2023-07-19 21:24 ` Larry Finger 2023-07-19 22:37 ` Kees Cook 2023-07-20 14:51 ` Larry Finger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox