| Message ID | 1366965244-20542-2-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Apr 26, 2013 at 04:34:03PM +0800, Jason Wang wrote: > virtio-rng-ccw has zero config length, so we need validate the config length > before trying to access it. Otherwise we may crash since vdev->config is NULL. > > Cc: Cornelia Huck <cornelia.huck@de.ibm.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Alexander Graf <agraf@suse.de> > Signed-off-by: Jason Wang <jasowang@redhat.com> The real problem is dev->vdev->get_config being NULL, isn't it? So why not validate it and be done with it? > --- > hw/s390x/virtio-ccw.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c > index 56539d3..8d0dff5 100644 > --- a/hw/s390x/virtio-ccw.c > +++ b/hw/s390x/virtio-ccw.c > @@ -260,7 +260,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > } > } > len = MIN(ccw.count, dev->vdev->config_len); > - if (!ccw.cda) { > + if (!ccw.cda || !len) { > ret = -EFAULT; > } else { > dev->vdev->get_config(dev->vdev, dev->vdev->config); > @@ -279,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) > } > len = MIN(ccw.count, dev->vdev->config_len); > hw_len = len; > - if (!ccw.cda) { > + if (!ccw.cda || !len) { > ret = -EFAULT; > } else { > config = cpu_physical_memory_map(ccw.cda, &hw_len, 0); > -- > 1.7.1
On 04/28/2013 04:32 PM, Michael S. Tsirkin wrote: > On Fri, Apr 26, 2013 at 04:34:03PM +0800, Jason Wang wrote: >> virtio-rng-ccw has zero config length, so we need validate the config length >> before trying to access it. Otherwise we may crash since vdev->config is NULL. >> >> Cc: Cornelia Huck <cornelia.huck@de.ibm.com> >> Cc: Richard Henderson <rth@twiddle.net> >> Cc: Alexander Graf <agraf@suse.de> >> Signed-off-by: Jason Wang <jasowang@redhat.com> > The real problem is dev->vdev->get_config being NULL, > isn't it? So why not validate it and be done with it? Ok, this looks more clear. Will do it in V2. >> --- >> hw/s390x/virtio-ccw.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >> index 56539d3..8d0dff5 100644 >> --- a/hw/s390x/virtio-ccw.c >> +++ b/hw/s390x/virtio-ccw.c >> @@ -260,7 +260,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) >> } >> } >> len = MIN(ccw.count, dev->vdev->config_len); >> - if (!ccw.cda) { >> + if (!ccw.cda || !len) { >> ret = -EFAULT; >> } else { >> dev->vdev->get_config(dev->vdev, dev->vdev->config); >> @@ -279,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) >> } >> len = MIN(ccw.count, dev->vdev->config_len); >> hw_len = len; >> - if (!ccw.cda) { >> + if (!ccw.cda || !len) { >> ret = -EFAULT; >> } else { >> config = cpu_physical_memory_map(ccw.cda, &hw_len, 0); >> -- >> 1.7.1
On 04/28/2013 04:40 PM, Jason Wang wrote: > On 04/28/2013 04:32 PM, Michael S. Tsirkin wrote: >> On Fri, Apr 26, 2013 at 04:34:03PM +0800, Jason Wang wrote: >>> virtio-rng-ccw has zero config length, so we need validate the config length >>> before trying to access it. Otherwise we may crash since vdev->config is NULL. >>> >>> Cc: Cornelia Huck <cornelia.huck@de.ibm.com> >>> Cc: Richard Henderson <rth@twiddle.net> >>> Cc: Alexander Graf <agraf@suse.de> >>> Signed-off-by: Jason Wang <jasowang@redhat.com> >> The real problem is dev->vdev->get_config being NULL, >> isn't it? So why not validate it and be done with it? > Ok, this looks more clear. Will do it in V2. Recheck the code, looks like {get|set}_config() has been validated in virtio_bus_get_vdev_config(). So the codes were ok here, will drop this patch also. >>> --- >>> hw/s390x/virtio-ccw.c | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c >>> index 56539d3..8d0dff5 100644 >>> --- a/hw/s390x/virtio-ccw.c >>> +++ b/hw/s390x/virtio-ccw.c >>> @@ -260,7 +260,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) >>> } >>> } >>> len = MIN(ccw.count, dev->vdev->config_len); >>> - if (!ccw.cda) { >>> + if (!ccw.cda || !len) { >>> ret = -EFAULT; >>> } else { >>> dev->vdev->get_config(dev->vdev, dev->vdev->config); >>> @@ -279,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) >>> } >>> len = MIN(ccw.count, dev->vdev->config_len); >>> hw_len = len; >>> - if (!ccw.cda) { >>> + if (!ccw.cda || !len) { >>> ret = -EFAULT; >>> } else { >>> config = cpu_physical_memory_map(ccw.cda, &hw_len, 0); >>> -- >>> 1.7.1 >
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index 56539d3..8d0dff5 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -260,7 +260,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } } len = MIN(ccw.count, dev->vdev->config_len); - if (!ccw.cda) { + if (!ccw.cda || !len) { ret = -EFAULT; } else { dev->vdev->get_config(dev->vdev, dev->vdev->config); @@ -279,7 +279,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) } len = MIN(ccw.count, dev->vdev->config_len); hw_len = len; - if (!ccw.cda) { + if (!ccw.cda || !len) { ret = -EFAULT; } else { config = cpu_physical_memory_map(ccw.cda, &hw_len, 0);
virtio-rng-ccw has zero config length, so we need validate the config length before trying to access it. Otherwise we may crash since vdev->config is NULL. Cc: Cornelia Huck <cornelia.huck@de.ibm.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Alexander Graf <agraf@suse.de> Signed-off-by: Jason Wang <jasowang@redhat.com> --- hw/s390x/virtio-ccw.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)