vbox6: fix "Error: slab block [xxx] is corrupt"

In 'SUPR3InitEx' (SUPLib.cpp) a 'SUPQUERYFUNCS' structure is allocated
with

!  (PSUPQUERYFUNCS)RTMemAllocZ(SUP_IOCTL_QUERY_FUNCS_SIZE(CookieReq.u.Out.cFunctions));

where 'CookieReq.u.Out.cFunctions' is 0. To determine the size of the
allocation

! #define SUP_IOCTL_QUERY_FUNCS_SIZE(cFuncs) \
!   RT_UOFFSETOF_DYN(SUPQUERYFUNCS, u.Out.aFunctions[(cFuncs)])

is used with cFuncs = 0 (SUPDrvIOC.h) leading to an allocation up to the
arrow below

! typedef struct SUPQUERYFUNCS
! {
!     /** The header. */
!     SUPREQHDR               Hdr;
!     union
!     {
!         struct
!         {
!             /** Number of functions returned. */
!             uint32_t        cFunctions;
!             /** Array of functions. */
==> end of allocation
!             SUPFUNC         aFunctions[1];
!         } Out;
!     } u;
==> sizeof(SUPQUERYFUNCS)
! } SUPQUERYFUNCS, *PSUPQUERYFUNCS;

In sup.cc (Genode) 'ioctl(SUPQUERYFUNCS &request)' will lead to
'with_out_ioctl'

! auto &out = request.u.Out;

where auto is 'SUPQUERYFUNCS' and finally

! out = { };

will zero out 'SUPQUERYFUNCS' up to the second arrow above. Because
'RTMemAllocZ' will call 'calloc' to allocate the memory 'out = { };'
will corrupt the slab block after the allocation. Therefore, it is
reasonable to allocate at least 'sizeof(SUPQUERYFUNCS)'.

Note there might be other 'ioctl' cases like this. A better way might be
to use 'SUPQUERYFUNCS.Hdr.cbOut' to determine the 'out' size.

fixes #4675
This commit is contained in:
Sebastian Sumpf 2023-05-18 21:22:03 +02:00 committed by Christian Helmuth
parent 40f31a9050
commit 4903487f21
3 changed files with 19 additions and 1 deletions

View File

@ -1 +1 @@
f5ab8d8b5f20dc67de05dc2cb7abd2b9cc1c8b6c
f5ebf8e03d4b29d3c186212bdc2d324bb31c5274

View File

@ -11,3 +11,4 @@ svga.patch
audio.patch
gcc-12.patch
pgmphys.patch
sup_ioctl_query_func_size.patch

View File

@ -0,0 +1,17 @@
index fa26845..c7377e8 100644
--- a/src/virtualbox6/src/VBox/HostDrivers/Support/SUPDrvIOC.h
+++ b/src/virtualbox6/src/VBox/HostDrivers/Support/SUPDrvIOC.h
@@ -267,7 +267,12 @@ typedef struct SUPCOOKIE
* @{
*/
#define SUP_IOCTL_QUERY_FUNCS(cFuncs) SUP_CTL_CODE_BIG(2)
-#define SUP_IOCTL_QUERY_FUNCS_SIZE(cFuncs) RT_UOFFSETOF_DYN(SUPQUERYFUNCS, u.Out.aFunctions[(cFuncs)])
+/*
+ * Ensure to return offset of at least 'aFunctions[1]' because the array in
+ * SUPQUERYFUNC has one element. Anyone using 'sizeof(SUPQUERYFUNCS)' (e.g.,
+ * constructor) will still access aFunctions[0] in case 'cFuncs' is 0.
+ */
+#define SUP_IOCTL_QUERY_FUNCS_SIZE(cFuncs) RT_UOFFSETOF_DYN(SUPQUERYFUNCS, u.Out.aFunctions[(cFuncs ? cFuncs : 1)])
#define SUP_IOCTL_QUERY_FUNCS_SIZE_IN sizeof(SUPREQHDR)
#define SUP_IOCTL_QUERY_FUNCS_SIZE_OUT(cFuncs) SUP_IOCTL_QUERY_FUNCS_SIZE(cFuncs)