linux-kernel - [PATCH 1/1] virtio: write back F_VERSION

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-next>] [day] [month] [year] [list]

Message-Id: <20211006142533.2735019-1-pasic@linux.ibm.com>
Date:   Wed,  6 Oct 2021 16:25:33 +0200
From:   Halil Pasic <pasic@...ux.ibm.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Xie Yongji <xieyongji@...edance.com>,
        virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Cc:     Halil Pasic <pasic@...ux.ibm.com>, markver@...ibm.com,
        Cornelia Huck <cohuck@...hat.com>,
        Christian Borntraeger <borntraeger@...ibm.com>,
        linux-s390@...r.kernel.org, stefanha@...hat.com,
        Raphael Norwitz <raphael.norwitz@...anix.com>,
        qemu-devel@...gnu.org
Subject: [PATCH 1/1] virtio: write back F_VERSION_1 before validate

The virtio specification virtio-v1.1-cs01 states: Transitional devices
MUST detect Legacy drivers by detecting that VIRTIO_F_VERSION_1 has not
been acknowledged by the driver.  This is exactly what QEMU as of 6.1
has done relying solely on VIRTIO_F_VERSION_1 for detecting that.

However, the specification also says: driver MAY read (but MUST NOT
write) the device-specific configuration fields to check that it can
support the device before setting FEATURES_OK.

In that case, any transitional device relying solely on
VIRTIO_F_VERSION_1 for detecting legacy drivers will return data in
legacy format.  In particular, this implies that it is in big endian
format for big endian guests. This naturally confuses the driver which
expects little endian in the modern mode.

It is probably a good idea to amend the spec to clarify that
VIRTIO_F_VERSION_1 can only be relied on after the feature negotiation
is complete. However, we already have regression so let's try to address
it.

Signed-off-by: Halil Pasic <pasic@...ux.ibm.com>
Fixes: 82e89ea077b9 ("virtio-blk: Add validation for block size in config space")
Fixes: fe36cbe0671e ("virtio_net: clear MTU when out of range")
Reported-by: markver@...ibm.com
---
 drivers/virtio/virtio.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 0a5b54034d4b..494cfecd3376 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -239,6 +239,16 @@ static int virtio_dev_probe(struct device *_d)
 		driver_features_legacy = driver_features;
 	}

+	/*
+	 * Some devices detect legacy solely via F_VERSION_1. Write
+	 * F_VERSION_1 to force LE for these when needed.
+	 */
+	if (drv->validate && !virtio_legacy_is_little_endian()
+			  && BIT_ULL(VIRTIO_F_VERSION_1) & device_features) {
+		dev->features = BIT_ULL(VIRTIO_F_VERSION_1);
+		dev->config->finalize_features(dev);
+	}
+
 	if (device_features & (1ULL << VIRTIO_F_VERSION_1))
 		dev->features = driver_features & device_features;
 	else

base-commit: 60a9483534ed0d99090a2ee1d4bb0b8179195f51
-- 
2.25.1