linux-s390.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Alexander Graf <agraf@suse.de>, KVM <kvm@vger.kernel.org>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	Jens Freimann <jfrei@linux.vnet.ibm.com>,
	linux-s390 <linux-s390@vger.kernel.org>,
	mst@redhat.com, Eric Farman <farman@linux.vnet.ibm.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/1] KVM: s390: virtio-ccw: don't overwrite config space values
Date: Wed, 1 Jul 2015 15:36:13 +0200	[thread overview]
Message-ID: <5593ECCD.8090702@redhat.com> (raw)
In-Reply-To: <1435589041-36194-2-git-send-email-borntraeger@de.ibm.com>



On 29/06/2015 16:44, Christian Borntraeger wrote:
> From: Cornelia Huck <cornelia.huck@de.ibm.com>
> 
> 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@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
> Reviewed-by: Eric Farman <farman@linux.vnet.ibm.com>
> Tested-by: Eric Farman <farman@linux.vnet.ibm.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: stable@vger.kernel.org
> ---
>  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

  reply	other threads:[~2015-07-01 13:36 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-29 14:44 [PATCH 0/1] KVM: s390: virtio-ccw: Fix config space values Christian Borntraeger
2015-06-29 14:44 ` [PATCH 1/1] KVM: s390: virtio-ccw: don't overwrite " Christian Borntraeger
2015-07-01 13:36   ` Paolo Bonzini [this message]
2015-07-01 13:45 ` [PATCH 0/1] KVM: s390: virtio-ccw: Fix " Michael S. Tsirkin
2015-07-01 14:05   ` Paolo Bonzini
2015-07-01 14:18     ` Michael S. Tsirkin
2015-07-01 14:21       ` Paolo Bonzini
2015-07-01 15:15         ` [PATCH] MAINTAINERS: separate section for s390 virtio drivers Cornelia Huck
2015-07-01 15:17           ` Paolo Bonzini
2015-07-07  9:41             ` [PATCH] virtio/s390: rename drivers/s390/kvm -> drivers/s390/virtio Cornelia Huck
2015-07-07  9:44             ` [PATCH] MAINTAINERS: separate section for s390 virtio drivers Cornelia Huck
2015-07-07 11:19               ` Michael S. Tsirkin
2015-07-02  9:45           ` 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=5593ECCD.8090702@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=agraf@suse.de \
    --cc=borntraeger@de.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=farman@linux.vnet.ibm.com \
    --cc=jfrei@linux.vnet.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=stable@vger.kernel.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 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).