All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Shishkin <alexander.shishkin@linux.intel.com>
To: mst@redhat.com, jasowang@redhat.com
Cc: virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, elena.reshetova@intel.com,
	kirill.shutemov@linux.intel.com,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Amit Shah <amit@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: [PATCH v1 4/6] virtio console: Harden control message handling
Date: Thu, 19 Jan 2023 15:57:19 +0200	[thread overview]
Message-ID: <20230119135721.83345-5-alexander.shishkin@linux.intel.com> (raw)
In-Reply-To: <20230119135721.83345-1-alexander.shishkin@linux.intel.com>

In handle_control_message(), we look at the ->event field twice, which
gives a malicious VMM a window in which to switch it from PORT_ADD to
PORT_REMOVE, triggering a null dereference further down the line:

RIP: 0010:spin_lock_irq ./include/linux/spinlock.h:388
RIP: 0010:unplug_port+0x9/0x150 drivers/char/virtio_console.c:1512
Call Trace:
 handle_control_message+0x108/0x2c0 drivers/char/virtio_console.c:1600
 elfcorehdr_read+0x40/0x40 ??:?
 process_one_work+0x1b4/0x310 kernel/workqueue.c:2297
 worker_thread+0x5c/0x3a0 kernel/workqueue.c:2444
 kthread+0x120/0x140 kernel/kthread.c:319
 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295

Read the event code once instead, basing all following decisions on the
same value.

Signed-off-by: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Amit Shah <amit@kernel.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/char/virtio_console.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 6599c2956ba4..62f69f949cb7 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1563,22 +1563,22 @@ static void handle_control_message(struct virtio_device *vdev,
 	struct port *port;
 	size_t name_size;
 	int err;
-	unsigned id;
+	unsigned id, event;
 
 	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
 
-	/* Make sure the host cannot change id under us */
+	/* Make sure the host cannot change id or event under us */
 	id = virtio32_to_cpu(vdev, READ_ONCE(cpkt->id));
+	event = virtio16_to_cpu(vdev, cpkt->event);
 	port = find_port_by_id(portdev, id);
-	if (!port &&
-	    cpkt->event != cpu_to_virtio16(vdev, VIRTIO_CONSOLE_PORT_ADD)) {
+	if (!port && event != VIRTIO_CONSOLE_PORT_ADD) {
 		/* No valid header at start of buffer.  Drop it. */
 		dev_dbg(&portdev->vdev->dev,
 			"Invalid index %u in control packet\n", cpkt->id);
 		return;
 	}
 
-	switch (virtio16_to_cpu(vdev, cpkt->event)) {
+	switch (event) {
 	case VIRTIO_CONSOLE_PORT_ADD:
 		if (port) {
 			dev_dbg(&portdev->vdev->dev,
-- 
2.39.0


  parent reply	other threads:[~2023-01-20  4:36 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-19 13:57 [PATCH v1 0/6] Harden a few virtio bits Alexander Shishkin
2023-01-19 13:57 ` [PATCH v1 1/6] virtio console: Harden multiport against invalid host input Alexander Shishkin
2023-01-19 15:17   ` Greg Kroah-Hartman
2023-01-19 15:17     ` Greg Kroah-Hartman
2023-01-19 18:52     ` Alexander Shishkin
2023-01-19 19:18       ` Greg Kroah-Hartman
2023-01-19 19:18         ` Greg Kroah-Hartman
2023-01-19 19:34         ` Alexander Shishkin
2023-01-20 13:01   ` Michael S. Tsirkin
2023-01-20 13:01     ` Michael S. Tsirkin
2023-01-20 15:51     ` Alexander Shishkin
2023-01-19 13:57 ` [PATCH v1 2/6] virtio console: Harden port adding Alexander Shishkin
2023-01-19 15:20   ` Greg Kroah-Hartman
2023-01-19 15:20     ` Greg Kroah-Hartman
2023-01-19 17:48     ` Alexander Shishkin
2023-01-19 18:57       ` Greg Kroah-Hartman
2023-01-19 18:57         ` Greg Kroah-Hartman
2023-01-19 20:13         ` Alexander Shishkin
2023-01-20  7:15           ` Greg Kroah-Hartman
2023-01-20  7:15             ` Greg Kroah-Hartman
2023-01-27 11:02           ` Michael S. Tsirkin
2023-01-27 11:02             ` Michael S. Tsirkin
2023-01-27 11:55             ` Alexander Shishkin
2023-01-27 12:12               ` Michael S. Tsirkin
2023-01-27 12:12                 ` Michael S. Tsirkin
2023-01-27 12:47                 ` Alexander Shishkin
2023-01-27 13:31                   ` Greg Kroah-Hartman
2023-01-27 13:31                     ` Greg Kroah-Hartman
2023-01-27 14:17                     ` Alexander Shishkin
2023-01-27 14:37                       ` Greg Kroah-Hartman
2023-01-27 14:37                         ` Greg Kroah-Hartman
2023-01-27 14:46                       ` Michael S. Tsirkin
2023-01-27 14:46                         ` Michael S. Tsirkin
2023-02-02 12:02                         ` Reshetova, Elena
2023-01-27 13:52                   ` Michael S. Tsirkin
2023-01-27 13:52                     ` Michael S. Tsirkin
2023-01-20 12:59   ` Michael S. Tsirkin
2023-01-20 12:59     ` Michael S. Tsirkin
2023-01-19 13:57 ` [PATCH v1 3/6] virtio 9p: Fix an overflow Alexander Shishkin
2023-01-20 12:54   ` Michael S. Tsirkin
2023-01-20 12:54     ` Michael S. Tsirkin
2023-01-20 16:29     ` Alexander Shishkin
2023-01-19 13:57 ` Alexander Shishkin [this message]
2023-01-19 15:22   ` [PATCH v1 4/6] virtio console: Harden control message handling Greg Kroah-Hartman
2023-01-19 15:22     ` Greg Kroah-Hartman
2023-01-20 12:45     ` Michael S. Tsirkin
2023-01-20 12:45       ` Michael S. Tsirkin
2023-01-20 16:41       ` Alexander Shishkin
2023-01-27 10:58         ` Michael S. Tsirkin
2023-01-27 10:58           ` Michael S. Tsirkin
2023-01-27 12:04           ` Alexander Shishkin
2023-01-19 13:57 ` [PATCH v1 5/6] virtio_net: Guard against buffer length overflow in xdp_linearize_page() Alexander Shishkin
2023-01-20 13:09   ` Michael S. Tsirkin
2023-01-20 13:09     ` Michael S. Tsirkin
2023-01-19 13:57 ` [PATCH v1 6/6] virtio_ring: Prevent bounds check bypass on descriptor index Alexander Shishkin
2023-01-20 12:56   ` Michael S. Tsirkin
2023-01-20 12:56     ` Michael S. Tsirkin
2023-01-20 11:55 ` [PATCH v1 0/6] Harden a few virtio bits Michael S. Tsirkin
2023-01-20 11:55   ` Michael S. Tsirkin
2023-01-20 12:32   ` Alexander Shishkin
2023-01-20 12:40     ` Michael S. Tsirkin
2023-01-20 12:40       ` Michael S. Tsirkin

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20230119135721.83345-5-alexander.shishkin@linux.intel.com \
    --to=alexander.shishkin@linux.intel.com \
    --cc=amit@kernel.org \
    --cc=arnd@arndb.de \
    --cc=elena.reshetova@intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jasowang@redhat.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.