From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Halil Pasic <pasic@linux.ibm.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Jason Wang <jasowang@redhat.com>,
Xie Yongji <xieyongji@bytedance.com>,
virtualization@lists.linux-foundation.org,
linux-kernel@vger.kernel.org
Cc: markver@us.ibm.com, Cornelia Huck <cohuck@redhat.com>,
linux-s390@vger.kernel.org
Subject: Re: [RFC PATCH 1/1] virtio: write back features before verify
Date: Thu, 30 Sep 2021 10:04:23 +0200 [thread overview]
Message-ID: <bd0bc232-8527-a4c7-d9be-3b4541914412@de.ibm.com> (raw)
In-Reply-To: <20210930012049.3780865-1-pasic@linux.ibm.com>
Am 30.09.21 um 03:20 schrieb Halil Pasic:
> This patch fixes a regression introduced by commit 82e89ea077b9
> ("virtio-blk: Add validation for block size in config space") and
> enables similar checks in verify() on big endian platforms.
>
> The problem with checking multi-byte config fields in the verify
> callback, on big endian platforms, and with a possibly transitional
> device is the following. The verify() callback is called between
> config->get_features() and virtio_finalize_features(). That we have a
> device that offered F_VERSION_1 then we have the following options
> either the device is transitional, and then it has to present the legacy
> interface, i.e. a big endian config space until F_VERSION_1 is
> negotiated, or we have a non-transitional device, which makes
> F_VERSION_1 mandatory, and only implements the non-legacy interface and
> thus presents a little endian config space. Because at this point we
> can't know if the device is transitional or non-transitional, we can't
> know do we need to byte swap or not.
>
> The virtio spec explicitly states that the driver MAY read config
> between reading and writing the features so saying that first accessing
> the config before feature negotiation is done is not an option. The
> specification ain't clear about setting the features multiple times
> before FEATURES_OK, so I guess that should be fine.
>
> I don't consider this patch super clean, but frankly I don't think we
> have a ton of options. Another option that may or man not be cleaner,
> but is also IMHO much uglier is to figure out whether the device is
> transitional by rejecting _F_VERSION_1, then resetting it and proceeding
> according tho what we have figured out, hoping that the characteristics
> of the device didn't change.
>
> Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> Fixes: 82e89ea077b9 ("virtio-blk: Add validation for block size in config space")
> Reported-by: markver@us.ibm.com
To make sure that it lands there, meybe add
cc stable 5.14
> ---
> drivers/virtio/virtio.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 0a5b54034d4b..9dc3cfa17b1c 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -249,6 +249,10 @@ static int virtio_dev_probe(struct device *_d)
> if (device_features & (1ULL << i))
> __virtio_set_bit(dev, i);
>
> + /* Write back features before validate to know endianness */
> + if (device_features & (1ULL << VIRTIO_F_VERSION_1))
> + dev->config->finalize_features(dev);
> +
> if (drv->validate) {
> err = drv->validate(dev);
> if (err)
>
> base-commit: 02d5e016800d082058b3d3b7c3ede136cdc6ddcb
>
next prev parent reply other threads:[~2021-09-30 8:05 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-30 1:20 [RFC PATCH 1/1] virtio: write back features before verify Halil Pasic
2021-09-30 8:04 ` Christian Borntraeger [this message]
2021-09-30 9:28 ` Cornelia Huck
2021-09-30 11:03 ` Halil Pasic
2021-09-30 11:31 ` Cornelia Huck
2021-10-01 14:22 ` Halil Pasic
2021-10-01 15:18 ` Cornelia Huck
2021-10-02 18:13 ` Michael S. Tsirkin
2021-10-04 2:23 ` Halil Pasic
2021-10-04 9:07 ` Michael S. Tsirkin
2021-10-05 10:06 ` Cornelia Huck
2021-10-05 10:43 ` Halil Pasic
2021-10-05 11:11 ` Michael S. Tsirkin
2021-10-05 11:13 ` Cornelia Huck
2021-10-05 11:20 ` Michael S. Tsirkin
2021-10-05 11:59 ` Halil Pasic
2021-10-05 15:25 ` Cornelia Huck
2021-10-04 7:01 ` Cornelia Huck
2021-10-04 9:25 ` Halil Pasic
2021-10-04 9:51 ` Cornelia Huck
2021-10-02 12:09 ` Michael S. Tsirkin
2021-09-30 11:12 ` Michael S. Tsirkin
2021-09-30 11:36 ` Cornelia Huck
2021-10-02 18:20 ` Michael S. Tsirkin
2021-10-03 5:00 ` Halil Pasic
2021-10-03 6:42 ` Michael S. Tsirkin
2021-10-03 7:26 ` Michael S. Tsirkin
2021-10-04 12:01 ` Cornelia Huck
2021-10-04 12:54 ` Michael S. Tsirkin
2021-10-04 14:27 ` Cornelia Huck
2021-10-04 15:05 ` Michael S. Tsirkin
2021-10-04 15:45 ` [virtio-dev] " Cornelia Huck
2021-10-04 20:01 ` Michael S. Tsirkin
2021-10-05 7:38 ` Cornelia Huck
2021-10-05 11:17 ` Halil Pasic
2021-10-05 11:22 ` Michael S. Tsirkin
2021-10-05 15:20 ` Cornelia Huck
2021-10-01 7:21 ` Halil Pasic
2021-10-02 10:21 ` Michael S. Tsirkin
2021-10-04 12:19 ` Cornelia Huck
2021-10-04 13:11 ` Michael S. Tsirkin
2021-10-04 14:33 ` Cornelia Huck
2021-10-04 15:07 ` Michael S. Tsirkin
2021-10-04 15:50 ` Cornelia Huck
2021-10-04 19:17 ` Michael S. Tsirkin
2021-10-06 10:13 ` Cornelia Huck
2021-10-06 12:15 ` Michael S. Tsirkin
2021-10-05 7:25 ` Halil Pasic
2021-10-05 7:53 ` Michael S. Tsirkin
2021-10-05 10:46 ` Halil Pasic
2021-10-05 11:11 ` Michael S. Tsirkin
2021-10-01 14:34 ` Christian Borntraeger
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=bd0bc232-8527-a4c7-d9be-3b4541914412@de.ibm.com \
--to=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=jasowang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=markver@us.ibm.com \
--cc=mst@redhat.com \
--cc=pasic@linux.ibm.com \
--cc=virtualization@lists.linux-foundation.org \
--cc=xieyongji@bytedance.com \
/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 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).