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

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

[Hans de Goede]

VirtualBox 6.0.x has a new feature where the guest kernel driver passes
info about the origin of the request (e.g. userspace or kernelspace) to
the hypervisor.

If we do not pass this information then when running the 6.0.x userspace
guest-additions tools on a 6.0.x host, some requests will get denied
with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and
the mounting of shared folders marked to be auto-mounted.

This commit implements passing the requestor info to the host, fixing this.

Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/virt/vboxguest/vboxguest_core.c    | 106 ++++++++++++++-------
 drivers/virt/vboxguest/vboxguest_core.h    |  15 +--
 drivers/virt/vboxguest/vboxguest_linux.c   |  26 ++++-
 drivers/virt/vboxguest/vboxguest_utils.c   |  32 ++++---
 drivers/virt/vboxguest/vboxguest_version.h |   9 +-
 drivers/virt/vboxguest/vmmdev.h            |   8 +-
 include/linux/vbox_utils.h                 |  12 ++-
 include/uapi/linux/vbox_vmmdev_types.h     |  31 ++++++
 8 files changed, 168 insertions(+), 71 deletions(-)

diff --git a/drivers/virt/vboxguest/vboxguest_core.c b/drivers/virt/vboxguest/vboxguest_core.c
index 1475ed5ffcde..2ec5b34ffed7 100644
--- a/drivers/virt/vboxguest/vboxguest_core.c
+++ b/drivers/virt/vboxguest/vboxguest_core.c
@@ -27,6 +27,10 @@
 
 #define GUEST_MAPPINGS_TRIES	5
 
+#define VBG_KERNEL_REQUEST \
+	(VMMDEV_REQUESTOR_KERNEL | VMMDEV_REQUESTOR_USR_DRV | \
+	 VMMDEV_REQUESTOR_CON_DONT_KNOW | VMMDEV_REQUESTOR_TRUST_NOT_GIVEN)
+
 /**
  * Reserves memory in which the VMM can relocate any guest mappings
  * that are floating around.
@@ -48,7 +52,8 @@ static void vbg_guest_mappings_init(struct vbg_dev *gdev)
 	int i, rc;
 
 	/* Query the required space. */
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HYPERVISOR_INFO);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HYPERVISOR_INFO,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return;
 
@@ -135,7 +140,8 @@ static void vbg_guest_mappings_exit(struct vbg_dev *gdev)
 	 * Tell the host that we're going to free the memory we reserved for
 	 * it, the free it up. (Leak the memory if anything goes wrong here.)
 	 */
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_HYPERVISOR_INFO);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_HYPERVISOR_INFO,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return;
 
@@ -172,8 +178,10 @@ static int vbg_report_guest_info(struct vbg_dev *gdev)
 	struct vmmdev_guest_info2 *req2 = NULL;
 	int rc, ret = -ENOMEM;
 
-	req1 = vbg_req_alloc(sizeof(*req1), VMMDEVREQ_REPORT_GUEST_INFO);
-	req2 = vbg_req_alloc(sizeof(*req2), VMMDEVREQ_REPORT_GUEST_INFO2);
+	req1 = vbg_req_alloc(sizeof(*req1), VMMDEVREQ_REPORT_GUEST_INFO,
+			     VBG_KERNEL_REQUEST);
+	req2 = vbg_req_alloc(sizeof(*req2), VMMDEVREQ_REPORT_GUEST_INFO2,
+			     VBG_KERNEL_REQUEST);
 	if (!req1 || !req2)
 		goto out_free;
 
@@ -187,8 +195,8 @@ static int vbg_report_guest_info(struct vbg_dev *gdev)
 	req2->additions_minor = VBG_VERSION_MINOR;
 	req2->additions_build = VBG_VERSION_BUILD;
 	req2->additions_revision = VBG_SVN_REV;
-	/* (no features defined yet) */
-	req2->additions_features = 0;
+	req2->additions_features =
+		VMMDEV_GUEST_INFO2_ADDITIONS_FEATURES_REQUESTOR_INFO;
 	strlcpy(req2->name, VBG_VERSION_STRING,
 		sizeof(req2->name));
 
@@ -230,7 +238,8 @@ static int vbg_report_driver_status(struct vbg_dev *gdev, bool active)
 	struct vmmdev_guest_status *req;
 	int rc;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_REPORT_GUEST_STATUS);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_REPORT_GUEST_STATUS,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
@@ -423,7 +432,8 @@ static int vbg_heartbeat_host_config(struct vbg_dev *gdev, bool enabled)
 	struct vmmdev_heartbeat *req;
 	int rc;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_HEARTBEAT_CONFIGURE);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_HEARTBEAT_CONFIGURE,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
@@ -457,7 +467,8 @@ static int vbg_heartbeat_init(struct vbg_dev *gdev)
 
 	gdev->guest_heartbeat_req = vbg_req_alloc(
 					sizeof(*gdev->guest_heartbeat_req),
-					VMMDEVREQ_GUEST_HEARTBEAT);
+					VMMDEVREQ_GUEST_HEARTBEAT,
+					VBG_KERNEL_REQUEST);
 	if (!gdev->guest_heartbeat_req)
 		return -ENOMEM;
 
@@ -528,7 +539,8 @@ static int vbg_reset_host_event_filter(struct vbg_dev *gdev,
 	struct vmmdev_mask *req;
 	int rc;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
@@ -567,8 +579,14 @@ static int vbg_set_session_event_filter(struct vbg_dev *gdev,
 	u32 changed, previous;
 	int rc, ret = 0;
 
-	/* Allocate a request buffer before taking the spinlock */
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK);
+	/*
+	 * Allocate a request buffer before taking the spinlock, when
+	 * the session is being terminated the requestor is the kernel,
+	 * as we're cleaning up.
+	 */
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_CTL_GUEST_FILTER_MASK,
+			    session_termination ? VBG_KERNEL_REQUEST :
+						  session->requestor);
 	if (!req) {
 		if (!session_termination)
 			return -ENOMEM;
@@ -627,7 +645,8 @@ static int vbg_reset_host_capabilities(struct vbg_dev *gdev)
 	struct vmmdev_mask *req;
 	int rc;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
@@ -662,8 +681,14 @@ static int vbg_set_session_capabilities(struct vbg_dev *gdev,
 	u32 changed, previous;
 	int rc, ret = 0;
 
-	/* Allocate a request buffer before taking the spinlock */
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES);
+	/*
+	 * Allocate a request buffer before taking the spinlock, when
+	 * the session is being terminated the requestor is the kernel,
+	 * as we're cleaning up.
+	 */
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_GUEST_CAPABILITIES,
+			    session_termination ? VBG_KERNEL_REQUEST :
+						  session->requestor);
 	if (!req) {
 		if (!session_termination)
 			return -ENOMEM;
@@ -722,7 +747,8 @@ static int vbg_query_host_version(struct vbg_dev *gdev)
 	struct vmmdev_host_version *req;
 	int rc, ret;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HOST_VERSION);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_GET_HOST_VERSION,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
@@ -783,19 +809,24 @@ int vbg_core_init(struct vbg_dev *gdev, u32 fixed_events)
 
 	gdev->mem_balloon.get_req =
 		vbg_req_alloc(sizeof(*gdev->mem_balloon.get_req),
-			      VMMDEVREQ_GET_MEMBALLOON_CHANGE_REQ);
+			      VMMDEVREQ_GET_MEMBALLOON_CHANGE_REQ,
+			      VBG_KERNEL_REQUEST);
 	gdev->mem_balloon.change_req =
 		vbg_req_alloc(sizeof(*gdev->mem_balloon.change_req),
-			      VMMDEVREQ_CHANGE_MEMBALLOON);
+			      VMMDEVREQ_CHANGE_MEMBALLOON,
+			      VBG_KERNEL_REQUEST);
 	gdev->cancel_req =
 		vbg_req_alloc(sizeof(*(gdev->cancel_req)),
-			      VMMDEVREQ_HGCM_CANCEL2);
+			      VMMDEVREQ_HGCM_CANCEL2,
+			      VBG_KERNEL_REQUEST);
 	gdev->ack_events_req =
 		vbg_req_alloc(sizeof(*gdev->ack_events_req),
-			      VMMDEVREQ_ACKNOWLEDGE_EVENTS);
+			      VMMDEVREQ_ACKNOWLEDGE_EVENTS,
+			      VBG_KERNEL_REQUEST);
 	gdev->mouse_status_req =
 		vbg_req_alloc(sizeof(*gdev->mouse_status_req),
-			      VMMDEVREQ_GET_MOUSE_STATUS);
+			      VMMDEVREQ_GET_MOUSE_STATUS,
+			      VBG_KERNEL_REQUEST);
 
 	if (!gdev->mem_balloon.get_req || !gdev->mem_balloon.change_req ||
 	    !gdev->cancel_req || !gdev->ack_events_req ||
@@ -892,9 +923,9 @@ void vbg_core_exit(struct vbg_dev *gdev)
  * vboxguest_linux.c calls this when userspace opens the char-device.
  * Return: A pointer to the new session or an ERR_PTR on error.
  * @gdev:		The Guest extension device.
- * @user:		Set if this is a session for the vboxuser device.
+ * @requestor:		VMMDEV_REQUESTOR_* flags
  */
-struct vbg_session *vbg_core_open_session(struct vbg_dev *gdev, bool user)
+struct vbg_session *vbg_core_open_session(struct vbg_dev *gdev, u32 requestor)
 {
 	struct vbg_session *session;
 
@@ -903,7 +934,7 @@ struct vbg_session *vbg_core_open_session(struct vbg_dev *gdev, bool user)
 		return ERR_PTR(-ENOMEM);
 
 	session->gdev = gdev;
-	session->user_session = user;
+	session->requestor = requestor;
 
 	return session;
 }
@@ -924,7 +955,9 @@ void vbg_core_close_session(struct vbg_session *session)
 		if (!session->hgcm_client_ids[i])
 			continue;
 
-		vbg_hgcm_disconnect(gdev, session->hgcm_client_ids[i], &rc);
+		/* requestor is kernel here, as we're cleaning up. */
+		vbg_hgcm_disconnect(gdev, VBG_KERNEL_REQUEST,
+				    session->hgcm_client_ids[i], &rc);
 	}
 
 	kfree(session);
@@ -1152,7 +1185,8 @@ static int vbg_req_allowed(struct vbg_dev *gdev, struct vbg_session *session,
 		return -EPERM;
 	}
 
-	if (trusted_apps_only && session->user_session) {
+	if (trusted_apps_only &&
+	    (session->requestor & VMMDEV_REQUESTOR_USER_DEVICE)) {
 		vbg_err("Denying userspace vmm call type %#08x through vboxuser device node\n",
 			req->request_type);
 		return -EPERM;
@@ -1209,8 +1243,8 @@ static int vbg_ioctl_hgcm_connect(struct vbg_dev *gdev,
 	if (i >= ARRAY_SIZE(session->hgcm_client_ids))
 		return -EMFILE;
 
-	ret = vbg_hgcm_connect(gdev, &conn->u.in.loc, &client_id,
-			       &conn->hdr.rc);
+	ret = vbg_hgcm_connect(gdev, session->requestor, &conn->u.in.loc,
+			       &client_id, &conn->hdr.rc);
 
 	mutex_lock(&gdev->session_mutex);
 	if (ret == 0 && conn->hdr.rc >= 0) {
@@ -1251,7 +1285,8 @@ static int vbg_ioctl_hgcm_disconnect(struct vbg_dev *gdev,
 	if (i >= ARRAY_SIZE(session->hgcm_client_ids))
 		return -EINVAL;
 
-	ret = vbg_hgcm_disconnect(gdev, client_id, &disconn->hdr.rc);
+	ret = vbg_hgcm_disconnect(gdev, session->requestor, client_id,
+				  &disconn->hdr.rc);
 
 	mutex_lock(&gdev->session_mutex);
 	if (ret == 0 && disconn->hdr.rc >= 0)
@@ -1313,12 +1348,12 @@ static int vbg_ioctl_hgcm_call(struct vbg_dev *gdev,
 	}
 
 	if (IS_ENABLED(CONFIG_COMPAT) && f32bit)
-		ret = vbg_hgcm_call32(gdev, client_id,
+		ret = vbg_hgcm_call32(gdev, session->requestor, client_id,
 				      call->function, call->timeout_ms,
 				      VBG_IOCTL_HGCM_CALL_PARMS32(call),
 				      call->parm_count, &call->hdr.rc);
 	else
-		ret = vbg_hgcm_call(gdev, client_id,
+		ret = vbg_hgcm_call(gdev, session->requestor, client_id,
 				    call->function, call->timeout_ms,
 				    VBG_IOCTL_HGCM_CALL_PARMS(call),
 				    call->parm_count, &call->hdr.rc);
@@ -1408,6 +1443,7 @@ static int vbg_ioctl_check_balloon(struct vbg_dev *gdev,
 }
 
 static int vbg_ioctl_write_core_dump(struct vbg_dev *gdev,
+				     struct vbg_session *session,
 				     struct vbg_ioctl_write_coredump *dump)
 {
 	struct vmmdev_write_core_dump *req;
@@ -1415,7 +1451,8 @@ static int vbg_ioctl_write_core_dump(struct vbg_dev *gdev,
 	if (vbg_ioctl_chk(&dump->hdr, sizeof(dump->u.in), 0))
 		return -EINVAL;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_WRITE_COREDUMP);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_WRITE_COREDUMP,
+			    session->requestor);
 	if (!req)
 		return -ENOMEM;
 
@@ -1476,7 +1513,7 @@ int vbg_core_ioctl(struct vbg_session *session, unsigned int req, void *data)
 	case VBG_IOCTL_CHECK_BALLOON:
 		return vbg_ioctl_check_balloon(gdev, data);
 	case VBG_IOCTL_WRITE_CORE_DUMP:
-		return vbg_ioctl_write_core_dump(gdev, data);
+		return vbg_ioctl_write_core_dump(gdev, session, data);
 	}
 
 	/* Variable sized requests. */
@@ -1508,7 +1545,8 @@ int vbg_core_set_mouse_status(struct vbg_dev *gdev, u32 features)
 	struct vmmdev_mouse_status *req;
 	int rc;
 
-	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_MOUSE_STATUS);
+	req = vbg_req_alloc(sizeof(*req), VMMDEVREQ_SET_MOUSE_STATUS,
+			    VBG_KERNEL_REQUEST);
 	if (!req)
 		return -ENOMEM;
 
diff --git a/drivers/virt/vboxguest/vboxguest_core.h b/drivers/virt/vboxguest/vboxguest_core.h
index 7ad9ec45bfa9..4188c12b839f 100644
--- a/drivers/virt/vboxguest/vboxguest_core.h
+++ b/drivers/virt/vboxguest/vboxguest_core.h
@@ -154,15 +154,15 @@ struct vbg_session {
 	 * host. Protected by vbg_gdev.session_mutex.
 	 */
 	u32 guest_caps;
-	/** Does this session belong to a root process or a user one? */
-	bool user_session;
+	/** VMMDEV_REQUESTOR_* flags */
+	u32 requestor;
 	/** Set on CANCEL_ALL_WAITEVENTS, protected by vbg_devevent_spinlock. */
 	bool cancel_waiters;
 };
 
 int  vbg_core_init(struct vbg_dev *gdev, u32 fixed_events);
 void vbg_core_exit(struct vbg_dev *gdev);
-struct vbg_session *vbg_core_open_session(struct vbg_dev *gdev, bool user);
+struct vbg_session *vbg_core_open_session(struct vbg_dev *gdev, u32 requestor);
 void vbg_core_close_session(struct vbg_session *session);
 int  vbg_core_ioctl(struct vbg_session *session, unsigned int req, void *data);
 int  vbg_core_set_mouse_status(struct vbg_dev *gdev, u32 features);
@@ -172,12 +172,13 @@ irqreturn_t vbg_core_isr(int irq, void *dev_id);
 void vbg_linux_mouse_event(struct vbg_dev *gdev);
 
 /* Private (non exported) functions form vboxguest_utils.c */
-void *vbg_req_alloc(size_t len, enum vmmdev_request_type req_type);
+void *vbg_req_alloc(size_t len, enum vmmdev_request_type req_type,
+		    u32 requestor);
 void vbg_req_free(void *req, size_t len);
 int vbg_req_perform(struct vbg_dev *gdev, void *req);
 int vbg_hgcm_call32(
-	struct vbg_dev *gdev, u32 client_id, u32 function, u32 timeout_ms,
-	struct vmmdev_hgcm_function_parameter32 *parm32, u32 parm_count,
-	int *vbox_status);
+	struct vbg_dev *gdev, u32 requestor, u32 client_id, u32 function,
+	u32 timeout_ms, struct vmmdev_hgcm_function_parameter32 *parm32,
+	u32 parm_count, int *vbox_status);
 
 #endif
diff --git a/drivers/virt/vboxguest/vboxguest_linux.c b/drivers/virt/vboxguest/vboxguest_linux.c
index 6e2a9619192d..6e8c0f1c1056 100644
--- a/drivers/virt/vboxguest/vboxguest_linux.c
+++ b/drivers/virt/vboxguest/vboxguest_linux.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2006-2016 Oracle Corporation
  */
 
+#include <linux/cred.h>
 #include <linux/input.h>
 #include <linux/kernel.h>
 #include <linux/miscdevice.h>
@@ -28,6 +29,23 @@ static DEFINE_MUTEX(vbg_gdev_mutex);
 /** Global vbg_gdev pointer used by vbg_get/put_gdev. */
 static struct vbg_dev *vbg_gdev;
 
+static u32 vbg_misc_device_requestor(struct inode *inode)
+{
+	u32 requestor = VMMDEV_REQUESTOR_USERMODE |
+			VMMDEV_REQUESTOR_CON_DONT_KNOW |
+			VMMDEV_REQUESTOR_TRUST_NOT_GIVEN;
+
+	if (from_kuid(current_user_ns(), current->cred->uid) == 0)
+		requestor |= VMMDEV_REQUESTOR_USR_ROOT;
+	else
+		requestor |= VMMDEV_REQUESTOR_USR_USER;
+
+	if (in_egroup_p(inode->i_gid))
+		requestor |= VMMDEV_REQUESTOR_GRP_VBOX;
+
+	return requestor;
+}
+
 static int vbg_misc_device_open(struct inode *inode, struct file *filp)
 {
 	struct vbg_session *session;
@@ -36,7 +54,7 @@ static int vbg_misc_device_open(struct inode *inode, struct file *filp)
 	/* misc_open sets filp->private_data to our misc device */
 	gdev = container_of(filp->private_data, struct vbg_dev, misc_device);
 
-	session = vbg_core_open_session(gdev, false);
+	session = vbg_core_open_session(gdev, vbg_misc_device_requestor(inode));
 	if (IS_ERR(session))
 		return PTR_ERR(session);
 
@@ -53,7 +71,8 @@ static int vbg_misc_device_user_open(struct inode *inode, struct file *filp)
 	gdev = container_of(filp->private_data, struct vbg_dev,
 			    misc_device_user);
 
-	session = vbg_core_open_session(gdev, false);
+	session = vbg_core_open_session(gdev, vbg_misc_device_requestor(inode) |
+					      VMMDEV_REQUESTOR_USER_DEVICE);
 	if (IS_ERR(session))
 		return PTR_ERR(session);
 
@@ -115,7 +134,8 @@ static long vbg_misc_device_ioctl(struct file *filp, unsigned int req,
 			 req == VBG_IOCTL_VMMDEV_REQUEST_BIG;
 
 	if (is_vmmdev_req)
-		buf = vbg_req_alloc(size, VBG_IOCTL_HDR_TYPE_DEFAULT);
+		buf = vbg_req_alloc(size, VBG_IOCTL_HDR_TYPE_DEFAULT,
+				    session->requestor);
 	else
 		buf = kmalloc(size, GFP_KERNEL);
 	if (!buf)
diff --git a/drivers/virt/vboxguest/vboxguest_utils.c b/drivers/virt/vboxguest/vboxguest_utils.c
index bf4474214b4d..75fd140b02ff 100644
--- a/drivers/virt/vboxguest/vboxguest_utils.c
+++ b/drivers/virt/vboxguest/vboxguest_utils.c
@@ -62,7 +62,8 @@ VBG_LOG(vbg_err, pr_err);
 VBG_LOG(vbg_debug, pr_debug);
 #endif
 
-void *vbg_req_alloc(size_t len, enum vmmdev_request_type req_type)
+void *vbg_req_alloc(size_t len, enum vmmdev_request_type req_type,
+		    u32 requestor)
 {
 	struct vmmdev_request_header *req;
 	int order = get_order(PAGE_ALIGN(len));
@@ -78,7 +79,7 @@ void *vbg_req_alloc(size_t len, enum vmmdev_request_type req_type)
 	req->request_type = req_type;
 	req->rc = VERR_GENERAL_FAILURE;
 	req->reserved1 = 0;
-	req->reserved2 = 0;
+	req->requestor = requestor;
 
 	return req;
 }
@@ -119,7 +120,7 @@ static bool hgcm_req_done(struct vbg_dev *gdev,
 	return done;
 }
 
-int vbg_hgcm_connect(struct vbg_dev *gdev,
+int vbg_hgcm_connect(struct vbg_dev *gdev, u32 requestor,
 		     struct vmmdev_hgcm_service_location *loc,
 		     u32 *client_id, int *vbox_status)
 {
@@ -127,7 +128,7 @@ int vbg_hgcm_connect(struct vbg_dev *gdev,
 	int rc;
 
 	hgcm_connect = vbg_req_alloc(sizeof(*hgcm_connect),
-				     VMMDEVREQ_HGCM_CONNECT);
+				     VMMDEVREQ_HGCM_CONNECT, requestor);
 	if (!hgcm_connect)
 		return -ENOMEM;
 
@@ -153,13 +154,15 @@ int vbg_hgcm_connect(struct vbg_dev *gdev,
 }
 EXPORT_SYMBOL(vbg_hgcm_connect);
 
-int vbg_hgcm_disconnect(struct vbg_dev *gdev, u32 client_id, int *vbox_status)
+int vbg_hgcm_disconnect(struct vbg_dev *gdev, u32 requestor,
+			u32 client_id, int *vbox_status)
 {
 	struct vmmdev_hgcm_disconnect *hgcm_disconnect = NULL;
 	int rc;
 
 	hgcm_disconnect = vbg_req_alloc(sizeof(*hgcm_disconnect),
-					VMMDEVREQ_HGCM_DISCONNECT);
+					VMMDEVREQ_HGCM_DISCONNECT,
+					requestor);
 	if (!hgcm_disconnect)
 		return -ENOMEM;
 
@@ -593,9 +596,10 @@ static int hgcm_call_copy_back_result(
 	return 0;
 }
 
-int vbg_hgcm_call(struct vbg_dev *gdev, u32 client_id, u32 function,
-		  u32 timeout_ms, struct vmmdev_hgcm_function_parameter *parms,
-		  u32 parm_count, int *vbox_status)
+int vbg_hgcm_call(struct vbg_dev *gdev, u32 requestor, u32 client_id,
+		  u32 function, u32 timeout_ms,
+		  struct vmmdev_hgcm_function_parameter *parms, u32 parm_count,
+		  int *vbox_status)
 {
 	struct vmmdev_hgcm_call *call;
 	void **bounce_bufs = NULL;
@@ -615,7 +619,7 @@ int vbg_hgcm_call(struct vbg_dev *gdev, u32 client_id, u32 function,
 		goto free_bounce_bufs;
 	}
 
-	call = vbg_req_alloc(size, VMMDEVREQ_HGCM_CALL);
+	call = vbg_req_alloc(size, VMMDEVREQ_HGCM_CALL, requestor);
 	if (!call) {
 		ret = -ENOMEM;
 		goto free_bounce_bufs;
@@ -647,9 +651,9 @@ EXPORT_SYMBOL(vbg_hgcm_call);
 
 #ifdef CONFIG_COMPAT
 int vbg_hgcm_call32(
-	struct vbg_dev *gdev, u32 client_id, u32 function, u32 timeout_ms,
-	struct vmmdev_hgcm_function_parameter32 *parm32, u32 parm_count,
-	int *vbox_status)
+	struct vbg_dev *gdev, u32 requestor, u32 client_id, u32 function,
+	u32 timeout_ms, struct vmmdev_hgcm_function_parameter32 *parm32,
+	u32 parm_count, int *vbox_status)
 {
 	struct vmmdev_hgcm_function_parameter *parm64 = NULL;
 	u32 i, size;
@@ -689,7 +693,7 @@ int vbg_hgcm_call32(
 			goto out_free;
 	}
 
-	ret = vbg_hgcm_call(gdev, client_id, function, timeout_ms,
+	ret = vbg_hgcm_call(gdev, requestor, client_id, function, timeout_ms,
 			    parm64, parm_count, vbox_status);
 	if (ret < 0)
 		goto out_free;
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 */
-#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"
 
 #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;
 };
 VMMDEV_ASSERT_SIZE(vmmdev_request_header, 24);
 
@@ -247,6 +247,8 @@ struct vmmdev_guest_info {
 };
 VMMDEV_ASSERT_SIZE(vmmdev_guest_info, 24 + 8);
 
+#define VMMDEV_GUEST_INFO2_ADDITIONS_FEATURES_REQUESTOR_INFO	BIT(0)
+
 /** struct vmmdev_guestinfo2 - Guest information report, version 2. */
 struct vmmdev_guest_info2 {
 	/** Header. */
@@ -259,7 +261,7 @@ struct vmmdev_guest_info2 {
 	u32 additions_build;
 	/** SVN revision. */
 	u32 additions_revision;
-	/** Feature mask, currently unused. */
+	/** Feature mask. */
 	u32 additions_features;
 	/**
 	 * The intentional meaning of this field was:
diff --git a/include/linux/vbox_utils.h b/include/linux/vbox_utils.h
index a240ed2a0372..ff56c443180c 100644
--- a/include/linux/vbox_utils.h
+++ b/include/linux/vbox_utils.h
@@ -24,15 +24,17 @@ __printf(1, 2) void vbg_debug(const char *fmt, ...);
 #define vbg_debug pr_debug
 #endif
 
-int vbg_hgcm_connect(struct vbg_dev *gdev,
+int vbg_hgcm_connect(struct vbg_dev *gdev, u32 requestor,
 		     struct vmmdev_hgcm_service_location *loc,
 		     u32 *client_id, int *vbox_status);
 
-int vbg_hgcm_disconnect(struct vbg_dev *gdev, u32 client_id, int *vbox_status);
+int vbg_hgcm_disconnect(struct vbg_dev *gdev, u32 requestor,
+			u32 client_id, int *vbox_status);
 
-int vbg_hgcm_call(struct vbg_dev *gdev, u32 client_id, u32 function,
-		  u32 timeout_ms, struct vmmdev_hgcm_function_parameter *parms,
-		  u32 parm_count, int *vbox_status);
+int vbg_hgcm_call(struct vbg_dev *gdev, u32 requestor, u32 client_id,
+		  u32 function, u32 timeout_ms,
+		  struct vmmdev_hgcm_function_parameter *parms, u32 parm_count,
+		  int *vbox_status);
 
 /**
  * Convert a VirtualBox status code to a standard Linux kernel return value.
diff --git a/include/uapi/linux/vbox_vmmdev_types.h b/include/uapi/linux/vbox_vmmdev_types.h
index 0e68024f36c7..8c535c2594ad 100644
--- 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
+/* 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
+/* Console classification mask. */
+#define VMMDEV_REQUESTOR_CON_MASK                           0x00000040
+/* Requestor is member of special VirtualBox user group. */
+#define VMMDEV_REQUESTOR_GRP_VBOX                           0x00000080
+/* Requestor trust level: Unspecified */
+#define VMMDEV_REQUESTOR_TRUST_NOT_GIVEN                    0x00000000
+/* Requestor trust level mask */
+#define VMMDEV_REQUESTOR_TRUST_MASK                         0x00007000
+/* Requestor is using the less trusted user device node (/dev/vboxuser) */
+#define VMMDEV_REQUESTOR_USER_DEVICE                        0x00008000
+
 /** HGCM service location types. */
 enum vmmdev_hgcm_service_location_type {
 	VMMDEV_HGCM_LOC_INVALID    = 0,
-- 
2.21.0

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

[Greg Kroah-Hartman]

On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote:
> VirtualBox 6.0.x has a new feature where the guest kernel driver passes
> info about the origin of the request (e.g. userspace or kernelspace) to
> the hypervisor.
> 
> If we do not pass this information then when running the 6.0.x userspace
> guest-additions tools on a 6.0.x host, some requests will get denied
> with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and
> the mounting of shared folders marked to be auto-mounted.
> 
> This commit implements passing the requestor info to the host, fixing this.
> 
> Cc: stable@vger.kernel.org

This feels like support for a "new feature", so why would this need to
go to older kernels?

It's not our fault that vb implemented a non-backwards-compatible change
for their new release, right?  So why should we be forced to add new
features to stable kernels?

I have no problem to add this for 5.2, but not for older stuff.

thanks,

greg k-h

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

[Hans de Goede]

Hi,

On 20-03-19 10:46, Greg Kroah-Hartman wrote:
> On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote:
>> VirtualBox 6.0.x has a new feature where the guest kernel driver passes
>> info about the origin of the request (e.g. userspace or kernelspace) to
>> the hypervisor.
>>
>> If we do not pass this information then when running the 6.0.x userspace
>> guest-additions tools on a 6.0.x host, some requests will get denied
>> with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and
>> the mounting of shared folders marked to be auto-mounted.
>>
>> This commit implements passing the requestor info to the host, fixing this.
>>
>> Cc: stable@vger.kernel.org
> 
> This feels like support for a "new feature", so why would this need to
> go to older kernels?
> 
> It's not our fault that vb implemented a non-backwards-compatible change
> for their new release, right?  So why should we be forced to add new
> features to stable kernels?

 From a technical point of view I completely agree with you and I'm unhappy
with this breakage after vb agreed with me to keep ABI compatibility so
that we could add a version of the vboxguest driver to the mainline kernel.

OTOH this is going to bite users out there, which is why I added the Cc:
stable. But this is entirely your call.

> I have no problem to add this for 5.2, but not for older stuff.

Can we at least at it as a fix to 5.1 ? It is not very adventurous.

Regards,

Hans

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

[Greg Kroah-Hartman]

On Wed, Mar 20, 2019 at 10:52:05AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 20-03-19 10:46, Greg Kroah-Hartman wrote:
> > On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote:
> > > VirtualBox 6.0.x has a new feature where the guest kernel driver passes
> > > info about the origin of the request (e.g. userspace or kernelspace) to
> > > the hypervisor.
> > > 
> > > If we do not pass this information then when running the 6.0.x userspace
> > > guest-additions tools on a 6.0.x host, some requests will get denied
> > > with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and
> > > the mounting of shared folders marked to be auto-mounted.
> > > 
> > > This commit implements passing the requestor info to the host, fixing this.
> > > 
> > > Cc: stable@vger.kernel.org
> > 
> > This feels like support for a "new feature", so why would this need to
> > go to older kernels?
> > 
> > It's not our fault that vb implemented a non-backwards-compatible change
> > for their new release, right?  So why should we be forced to add new
> > features to stable kernels?
> 
> From a technical point of view I completely agree with you and I'm unhappy
> with this breakage after vb agreed with me to keep ABI compatibility so
> that we could add a version of the vboxguest driver to the mainline kernel.

So they broke that agreement, ugh.  That implies they will do it again?

> OTOH this is going to bite users out there, which is why I added the Cc:
> stable. But this is entirely your call.

Let me think about it...

> > I have no problem to add this for 5.2, but not for older stuff.
> 
> Can we at least at it as a fix to 5.1 ? It is not very adventurous.

Sure, let me go review it now.

thanks,

greg k-h

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

[Greg Kroah-Hartman]

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

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

[Hans de Goede]

Hi,

On 20-03-19 19:19, Greg Kroah-Hartman wrote:
> On Wed, Mar 20, 2019 at 10:52:05AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 20-03-19 10:46, Greg Kroah-Hartman wrote:
>>> On Wed, Mar 20, 2019 at 10:35:19AM +0100, Hans de Goede wrote:
>>>> VirtualBox 6.0.x has a new feature where the guest kernel driver passes
>>>> info about the origin of the request (e.g. userspace or kernelspace) to
>>>> the hypervisor.
>>>>
>>>> If we do not pass this information then when running the 6.0.x userspace
>>>> guest-additions tools on a 6.0.x host, some requests will get denied
>>>> with a VERR_VERSION_MISMATCH error, breaking vboxservice.service and
>>>> the mounting of shared folders marked to be auto-mounted.
>>>>
>>>> This commit implements passing the requestor info to the host, fixing this.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>
>>> This feels like support for a "new feature", so why would this need to
>>> go to older kernels?
>>>
>>> It's not our fault that vb implemented a non-backwards-compatible change
>>> for their new release, right?  So why should we be forced to add new
>>> features to stable kernels?
>>
>>  From a technical point of view I completely agree with you and I'm unhappy
>> with this breakage after vb agreed with me to keep ABI compatibility so
>> that we could add a version of the vboxguest driver to the mainline kernel.
> 
> So they broke that agreement, ugh.  That implies they will do it again?

Well they did not really broke the ABI, they started using a reserved
field and a 6.0.x host will happily work with 5.2.x guest-extensions
(with the mainline vboxguest driver) the same way around, 5.2.x host
als works with 6.0.x guest-extensions user-space bits + mainline kernel
module.

Things break when using a 6.0.x host + 6.0.x guest-extensions userspace
parts combined with the mainline kernel module. The 6.0.x + 6.0.x combi
seems to assume that there is a 6.0.x vboxguest driver which fills in
the reserved field. I believe this is an oversight and not a deliberate
breakage. Perhaps this is even something which the VirtualBox devs can
fix in a future 6.0.x update...  Michael ?

>> OTOH this is going to bite users out there, which is why I added the Cc:
>> stable. But this is entirely your call.
> 
> Let me think about it...
> 
>>> I have no problem to add this for 5.2, but not for older stuff.
>>
>> Can we at least at it as a fix to 5.1 ? It is not very adventurous.
> 
> Sure, let me go review it now.

Thank you for the review, I will reply to it tomorrow (and prep a v2).

Regards,

Hans

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

[Hans de Goede]

Hi,

On 20-03-19 19:26, Greg Kroah-Hartman wrote:
> 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?

I did not sync with the latest version, I've picked
the svn revision number from the 6.0.0 release
(6.0.4 is out now) in case one of the 6.0.x releases
have introduced something new which we do not implement
yet.

>> -#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?

Unfortunately yes, last time I checked the host has
some code checking the VBG_SVN_REV to determine whether
or not to apply some bug workaround.

>>   
>>   #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 :(

There is another header for a hgcm-call which partly mirrors
this one and there they are using reserved1, I think it has
something to do with this.
>> --- 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?

The call coming directly from the call, rather then through the
/dev/vboxguest or /dev/vboxuser devices is indicated by the
lack of the VMMDEV_REQUESTOR_USERMODE bit. I will add a

#define VMMDEV_REQUESTOR_MODE_MASK				0x00000008

To make this more clear for v2 and add blank lines to group the different
items together per mask.


>> +/* 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?

VMMDEV_REQUESTOR_USR_NOT_GIVEN (the first 0x0000000 define) is part of the
which user made this call mask: VMMDEV_REQUESTOR_USR_MASK, this is part
of the is the user behind the physical console info mask:
VMMDEV_REQUESTOR_CON_MASK

I've omitted the other defines since we are not using them, I will add
them to make this more clear.


>> +/* 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?

Ack.


>> +/* 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?

Right, again I did not add defines for values we do not use under Linux
(this value comes from some Windows authentication mechanism, so under Linux
it is always VMMDEV_REQUESTOR_TRUST_NOT_GIVEN)


> 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?

There are 2 device nodes:

/dev/vboxguest
/dev/vboxuser

With the latter being mode 0666, the vboxguest code already disallows
some requests / IOCTLs when done on the /dev/vboxuser node.

With this new requestor mechanism, the purpose of which is to allow host
admins to write finer grained policies for which requests to accept from the
guest AFAIK, we are now also forwarding the info that the request was done
on the world accessible /dev/vboxuser node, rather then on the restricted
/dev/vboxguest node to the host.

Regards,

Hans