On Donnerstag, 29. Oktober 2020 09:25:41 CET Bin Meng wrote: > From: Bin Meng <[email protected]> > > At present the virtio device config space access is handled by the > virtio_config_readX() and virtio_config_writeX() APIs. They perform > a sanity check on the result of address plus size against the config > space size before the access occurs.
Since I am not very familiar with the virtio implementation side, I hope Michael would have a look at this patch. But some comments from my side ... > > For unaligned access, the last converted naturally aligned access > will fail the sanity check on 9pfs. For example, with a mount_tag > `p9fs`, if guest software tries to read the mount_tag via a 4 byte > read at the mount_tag offset which is not 4 byte aligned, the read > result will be `p9\377\377`, which is wrong. Why 4? Shouldn't this rather consider worst case alignment? > > This changes the size of device config space to be a multiple of 4 > bytes so that correct result can be returned in all circumstances. > > Signed-off-by: Bin Meng <[email protected]> > --- > > hw/9pfs/virtio-9p-device.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > index 14371a7..e6a1432 100644 > --- a/hw/9pfs/virtio-9p-device.c > +++ b/hw/9pfs/virtio-9p-device.c > @@ -201,6 +201,7 @@ static void virtio_9p_device_realize(DeviceState *dev, > Error **errp) > V9fsVirtioState *v = VIRTIO_9P(dev); > V9fsState *s = &v->state; > FsDriverEntry *fse = get_fsdev_fsentry(s->fsconf.fsdev_id); > + size_t config_size; > > if (qtest_enabled() && fse) { > fse->export_flags |= V9FS_NO_PERF_WARN; > @@ -211,7 +212,8 @@ static void virtio_9p_device_realize(DeviceState *dev, > Error **errp) > } > > v->config_size = sizeof(struct virtio_9p_config) + strlen(s->fsconf.tag); > - virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, v->config_size); > + config_size = ROUND_UP(v->config_size, 4); > + virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, config_size); > v->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output); > } Shouldn't this config_size correction rather be handled on virtio.c side instead, i.e. in virtio_init()? > > -- > 2.7.4 Best regards, Christian Schoenebeck
