On 29/06/2015 16:44, Christian Borntraeger wrote:
> From: Cornelia Huck <cornelia.huck@xxxxxxxxxx>
>
> Eric noticed problems with vhost-scsi and virtio-ccw: vhost-scsi
> complained about overwriting values in the config space, which
> was triggered by a broken implementation of virtio-ccw's config
> get/set routines. It was probably sheer luck that we did not hit
> this before.
>
> When writing a value to the config space, the WRITE_CONF ccw will
> always write from the beginning of the config space up to and
> including the value to be set. If the config space up to the value
> has not yet been retrieved from the device, however, we'll end up
> overwriting values. Keep track of the known config space and update
> if needed to avoid this.
>
> Moreover, READ_CONF will only read the number of bytes it has been
> instructed to retrieve, so we must not copy more than that to the
> buffer, or we might overwrite trailing values.
>
> Reported-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Cornelia Huck <cornelia.huck@xxxxxxxxxx>
> Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
> Tested-by: Eric Farman <farman@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> drivers/s390/kvm/virtio_ccw.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/s390/kvm/virtio_ccw.c b/drivers/s390/kvm/virtio_ccw.c
> index 6f1fa17..f8d8fdb 100644
> --- a/drivers/s390/kvm/virtio_ccw.c
> +++ b/drivers/s390/kvm/virtio_ccw.c
> @@ -65,6 +65,7 @@ struct virtio_ccw_device {
> bool is_thinint;
> bool going_away;
> bool device_lost;
> + unsigned int config_ready;
> void *airq_info;
> };
>
> @@ -833,8 +834,11 @@ static void virtio_ccw_get_config(struct virtio_device *vdev,
> if (ret)
> goto out_free;
>
> - memcpy(vcdev->config, config_area, sizeof(vcdev->config));
> - memcpy(buf, &vcdev->config[offset], len);
> + memcpy(vcdev->config, config_area, offset + len);
> + if (buf)
> + memcpy(buf, &vcdev->config[offset], len);
> + if (vcdev->config_ready < offset + len)
> + vcdev->config_ready = offset + len;
>
> out_free:
> kfree(config_area);
> @@ -857,6 +861,9 @@ static void virtio_ccw_set_config(struct virtio_device *vdev,
> if (!config_area)
> goto out_free;
>
> + /* Make sure we don't overwrite fields. */
> + if (vcdev->config_ready < offset)
> + virtio_ccw_get_config(vdev, 0, NULL, offset);
> memcpy(&vcdev->config[offset], buf, len);
> /* Write the config area to the host. */
> memcpy(config_area, vcdev->config, sizeof(vcdev->config));
>
Applied (but I think in general virtio-ccw patches should go through
mst---the exception is when matching changes to KVM are needed, and of
course the exception was almost always the rule during bringup).
Thanks,
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
