LKML Archive on
help / color / mirror / Atom feed
From: Wenwen Wang <>
To: Wenwen Wang <>
Cc: Kangjie Lu <>, Hans de Goede <>,
	Arnd Bergmann <>,
	Greg Kroah-Hartman <>, (open list)
Subject: [PATCH] virt: vbox: fix a missing-check bug
Date: Sat,  5 May 2018 22:30:48 -0500	[thread overview]
Message-ID: <> (raw)

In vbg_misc_device_ioctl(), the header of the ioctl argument is copied from
the userspace pointer 'arg' and saved to the kernel object 'hdr'. Then the
'version', 'size_in', and 'size_out' fields of 'hdr' are verified. For
example, if 'hdr.version' is not VBG_IOCTL_HDR_VERSION, an error code
-EINVAL will be returned. If 'hdr' can pass all verifications, the whole
structure of the ioctl argument is copied once again from 'arg' and saved
to 'buf'. Then the function vbg_core_ioctl() is invoked to execute the
ioctl command. Given that the 'arg' pointer resides in userspace, a
malicious userspace process can race to change the data pointed to by 'arg'
between the two copies. By doing so, the user can bypass the verifications
on the ioctl argument, which can cause vbg_core_ioctl() to work on unsecure
data because it assumes the 'version', 'size_in', and 'size_out' have been
verified by vbg_misc_device_ioctl(), as mentioned in the comment in

         * hdr->version and hdr->size_in / hdr->size_out minimum size are
         * already checked by vbg_misc_device_ioctl().

This patch adds checks after the second copy to ensure the consistency
between the data obtained in the two copies. In case an inconsistency is
detected, an error code -EINVAL will be returned.

Signed-off-by: Wenwen Wang <>
 drivers/virt/vboxguest/vboxguest_linux.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/virt/vboxguest/vboxguest_linux.c b/drivers/virt/vboxguest/vboxguest_linux.c
index 398d226..6b525259 100644
--- a/drivers/virt/vboxguest/vboxguest_linux.c
+++ b/drivers/virt/vboxguest/vboxguest_linux.c
@@ -125,6 +125,12 @@ static long vbg_misc_device_ioctl(struct file *filp, unsigned int req,
 		ret = -EFAULT;
 		goto out;
+	if (((struct vbg_ioctl_hdr *)buf)->version != hdr.version ||
+	    ((struct vbg_ioctl_hdr *)buf)->size_in != hdr.size_in ||
+	    ((struct vbg_ioctl_hdr *)buf)->size_out != hdr.size_out) {
+		ret = -EINVAL;
+		goto out;
+	}
 	if (hdr.size_in < size)
 		memset(buf + hdr.size_in, 0, size -  hdr.size_in);
@@ -133,11 +139,6 @@ static long vbg_misc_device_ioctl(struct file *filp, unsigned int req,
 		goto out;
 	returned_size = ((struct vbg_ioctl_hdr *)buf)->size_out;
-	if (returned_size > size) {
-		vbg_debug("%s: too much output data %zu > %zu\n",
-			  __func__, returned_size, size);
-		returned_size = size;
-	}
 	if (copy_to_user((void *)arg, buf, returned_size) != 0)
 		ret = -EFAULT;

             reply	other threads:[~2018-05-06  3:30 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-06  3:30 Wenwen Wang [this message]
2018-05-08 11:46 ` Hans de Goede
2018-05-08 12:52   ` Wenwen Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] virt: vbox: fix a missing-check bug' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).