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, Andi Kleen <ak@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 2/6] virtio console: Harden port adding
Date: Thu, 19 Jan 2023 15:57:17 +0200	[thread overview]
Message-ID: <20230119135721.83345-3-alexander.shishkin@linux.intel.com> (raw)
In-Reply-To: <20230119135721.83345-1-alexander.shishkin@linux.intel.com>

From: Andi Kleen <ak@linux.intel.com>

The ADD_PORT operation reads and sanity checks the port id multiple
times from the untrusted host. This is not safe because a malicious
host could change it between reads.

Read the port id only once and cache it for subsequent uses.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
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, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index f4fd5fe7cd3a..6599c2956ba4 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1563,10 +1563,13 @@ static void handle_control_message(struct virtio_device *vdev,
 	struct port *port;
 	size_t name_size;
 	int err;
+	unsigned id;
 
 	cpkt = (struct virtio_console_control *)(buf->buf + buf->offset);
 
-	port = find_port_by_id(portdev, virtio32_to_cpu(vdev, cpkt->id));
+	/* Make sure the host cannot change id under us */
+	id = virtio32_to_cpu(vdev, READ_ONCE(cpkt->id));
+	port = find_port_by_id(portdev, id);
 	if (!port &&
 	    cpkt->event != cpu_to_virtio16(vdev, VIRTIO_CONSOLE_PORT_ADD)) {
 		/* No valid header at start of buffer.  Drop it. */
@@ -1583,15 +1586,14 @@ static void handle_control_message(struct virtio_device *vdev,
 			send_control_msg(port, VIRTIO_CONSOLE_PORT_READY, 1);
 			break;
 		}
-		if (virtio32_to_cpu(vdev, cpkt->id) >=
-		    portdev->max_nr_ports) {
+		if (id >= portdev->max_nr_ports) {
 			dev_warn(&portdev->vdev->dev,
 				"Request for adding port with "
 				"out-of-bound id %u, max. supported id: %u\n",
 				cpkt->id, portdev->max_nr_ports - 1);
 			break;
 		}
-		add_port(portdev, virtio32_to_cpu(vdev, cpkt->id));
+		add_port(portdev, id);
 		break;
 	case VIRTIO_CONSOLE_PORT_REMOVE:
 		unplug_port(port);
-- 
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 ` Alexander Shishkin [this message]
2023-01-19 15:20   ` [PATCH v1 2/6] virtio console: Harden port adding 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 ` [PATCH v1 4/6] virtio console: Harden control message handling Alexander Shishkin
2023-01-19 15:22   ` 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-3-alexander.shishkin@linux.intel.com \
    --to=alexander.shishkin@linux.intel.com \
    --cc=ak@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.