linux-kernel - Re: [PATCH] virt: vbox: Implement passing requestor info to the host for VirtualBox 6.0.x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Mar 2019 19:26:14 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Michael Thayer <michael.thayer@...cle.com>,
        "Knut St . Osmundsen" <knut.osmundsen@...cle.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] virt: vbox: Implement passing requestor info to the host
 for VirtualBox 6.0.x

On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote:
> diff --git a/drivers/virt/vboxguest/vboxguest_version.h b/drivers/virt/vboxguest/vboxguest_version.h
> index 77f0c8f8a231..84834dad38d5 100644
> --- a/drivers/virt/vboxguest/vboxguest_version.h
> +++ b/drivers/virt/vboxguest/vboxguest_version.h
> @@ -9,11 +9,10 @@
>  #ifndef __VBOX_VERSION_H__
>  #define __VBOX_VERSION_H__
>  
> -/* Last synced October 4th 2017 */

We don't care about the date sync anymore?

> -#define VBG_VERSION_MAJOR 5
> -#define VBG_VERSION_MINOR 2
> +#define VBG_VERSION_MAJOR 6
> +#define VBG_VERSION_MINOR 0
>  #define VBG_VERSION_BUILD 0
> -#define VBG_SVN_REV 68940
> -#define VBG_VERSION_STRING "5.2.0"
> +#define VBG_SVN_REV 127566
> +#define VBG_VERSION_STRING "6.0.0"

Do we really need to keep track of this?

>  
>  #endif
> diff --git a/drivers/virt/vboxguest/vmmdev.h b/drivers/virt/vboxguest/vmmdev.h
> index 5e2ae978935d..6337b8d75d96 100644
> --- a/drivers/virt/vboxguest/vmmdev.h
> +++ b/drivers/virt/vboxguest/vmmdev.h
> @@ -98,8 +98,8 @@ struct vmmdev_request_header {
>  	s32 rc;
>  	/** Reserved field no.1. MBZ. */
>  	u32 reserved1;
> -	/** Reserved field no.2. MBZ. */
> -	u32 reserved2;
> +	/** IN: Requestor information (VMMDEV_REQUESTOR_*) */
> +	u32 requestor;

They didn't use the first reserved field?  Oh well :(

> --- a/include/uapi/linux/vbox_vmmdev_types.h
> +++ b/include/uapi/linux/vbox_vmmdev_types.h
> @@ -102,6 +102,37 @@ enum vmmdev_request_type {
>  #define VMMDEVREQ_HGCM_CALL VMMDEVREQ_HGCM_CALL32
>  #endif
>  
> +/* vmmdev_request_header.requestor defines */
> +
> +/* Requestor user not given. */
> +#define VMMDEV_REQUESTOR_USR_NOT_GIVEN                      0x00000000
> +/* The kernel driver (VBoxGuest) is the requestor. */
> +#define VMMDEV_REQUESTOR_USR_DRV                            0x00000001
> +/* Some other kernel driver is the requestor. */
> +#define VMMDEV_REQUESTOR_USR_DRV_OTHER                      0x00000002
> +/* The root or a admin user is the requestor. */
> +#define VMMDEV_REQUESTOR_USR_ROOT                           0x00000003
> +/* Regular joe user is making the request. */
> +#define VMMDEV_REQUESTOR_USR_USER                           0x00000006
> +/* User classification mask. */
> +#define VMMDEV_REQUESTOR_USR_MASK                           0x00000007
> +/* Kernel mode request. */
> +#define VMMDEV_REQUESTOR_KERNEL                             0x00000000

Wait, isn't that the same as VMMDEV_REQUESTOR_USR_NOT_GIVEN?

> +/* User mode request. */
> +#define VMMDEV_REQUESTOR_USERMODE                           0x00000008
> +/* Don't know the physical console association of the requestor. */
> +#define VMMDEV_REQUESTOR_CON_DONT_KNOW                      0x00000000

And here, why is this number recycled?

> +/* Console classification mask. */
> +#define VMMDEV_REQUESTOR_CON_MASK                           0x00000040
> +/* Requestor is member of special VirtualBox user group. */
> +#define VMMDEV_REQUESTOR_GRP_VBOX                           0x00000080

Can you add a blank line here to make it more obvious it is "TRUST"
stuff here?

> +/* Requestor trust level: Unspecified */
> +#define VMMDEV_REQUESTOR_TRUST_NOT_GIVEN                    0x00000000
> +/* Requestor trust level mask */
> +#define VMMDEV_REQUESTOR_TRUST_MASK                         0x00007000

So those bits are the "trust" values?

that's odd, oh well, it's their api, not ours :(

> +/* Requestor is using the less trusted user device node (/dev/vboxuser) */
> +#define VMMDEV_REQUESTOR_USER_DEVICE                        0x00008000

Wait, what is this for?

So the dev node isn't as "trusted" as some other api?  Why not?

confused,

greg k-h