Bug 1102668

Summary: Return value of virtio_load not checked in virtio_rng_load
Product: Red Hat Enterprise Linux 6 Reporter: Jeff Nelson <jen>
Component: qemu-kvmAssignee: Amos Kong <akong>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 6.6CC: ailan, bsarathy, chayang, juzhang, michen, mkenneth, qzhang, rbalakri, rhod, virt-maint
Target Milestone: rc   
Target Release: ---   
Hardware: All   
OS: All   
Whiteboard:
Fixed In Version: qemu-kvm-0.12.1.2-2.431.el6 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1116941 (view as bug list) Environment:
Last Closed: 2014-10-14 07:01:19 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1116941    

Description Jeff Nelson 2014-05-29 12:19:01 UTC
Covscan reports that there is a missing return value check on the call to virtio_load in routine virtio_load_rng in hw/virtio_rng.c:

    qemu-kvm-0.12.1.2/hw/virtio-rng.c:130: check_return: Calling function "virtio_load(VirtIODevice *, QEMUFile *)" without checking return value (as is done elsewhere 4 out of 5 times).
    qemu-kvm-0.12.1.2/hw/virtio-balloon.c:271: example_assign: Example 1: Assigning: "ret" = return value from "virtio_load(&s->vdev, f)".
    qemu-kvm-0.12.1.2/hw/virtio-balloon.c:272: example_checked: Example 1 (cont.): "ret" has its value checked in "ret".
    qemu-kvm-0.12.1.2/hw/virtio-net.c:1002: example_assign: Example 2: Assigning: "ret" = return value from "virtio_load(&n->vdev, f)".
    qemu-kvm-0.12.1.2/hw/virtio-net.c:1003: example_checked: Example 2 (cont.): "ret" has its value checked in "ret".
    qemu-kvm-0.12.1.2/hw/virtio-scsi.c:594: example_assign: Example 3: Assigning: "ret" = return value from "virtio_load(&s->vdev, f)".
    qemu-kvm-0.12.1.2/hw/virtio-scsi.c:595: example_checked: Example 3 (cont.): "ret" has its value checked in "ret".
    qemu-kvm-0.12.1.2/hw/virtio-serial-bus.c:670: example_assign: Example 4: Assigning: "ret" = return value from "virtio_load(&s->vdev, f)".
    qemu-kvm-0.12.1.2/hw/virtio-serial-bus.c:671: example_checked: Example 4 (cont.): "ret" has its value checked in "ret".
    qemu-kvm-0.12.1.2/hw/virtio-rng.c:130: unchecked_value: No check of the return value of "virtio_load(&vrng->vdev, f)".


Here is the patch that introduced the problem:

    commit 1ecb9c3580e9c6975a8f203555735499d7f8fc91
    Author: Amos Kong <akong>
    Date:   Tue Sep 10 06:07:55 2013 +0200

        virtio-rng: hardware random number generator device
    

Version-Release number of selected component (if applicable):
qemu-kvm-0.12.1.2-2.427.el6

How reproducible:
N/A

Comment 3 Amos Kong 2014-07-07 15:44:52 UTC
I will send a fix to upstream:

commit 689f2859c10671390e31a287b094a1bc02ffc015
Author: Amos Kong <akong>
Date:   Mon Jul 7 22:55:56 2014 +0800

    virtio-rng: check return value of virtio_load()
    
    Signed-off-by: Amos Kong <akong>

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index b6ab361..cc4b9e9 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -109,11 +109,16 @@ static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIORNG *vrng = opaque;
     VirtIODevice *vdev = VIRTIO_DEVICE(vrng);
+    int ret;
 
     if (version_id != 1) {
         return -EINVAL;
     }
-    virtio_load(vdev, f);
+
+    ret = virtio_load(vdev, f);
+    if (ret) {
+        return ret;
+    }
 
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may

Comment 4 Amos Kong 2014-07-07 15:44:53 UTC
I will send a fix to upstream:

commit 689f2859c10671390e31a287b094a1bc02ffc015
Author: Amos Kong <akong>
Date:   Mon Jul 7 22:55:56 2014 +0800

    virtio-rng: check return value of virtio_load()
    
    Signed-off-by: Amos Kong <akong>

diff --git a/hw/virtio/virtio-rng.c b/hw/virtio/virtio-rng.c
index b6ab361..cc4b9e9 100644
--- a/hw/virtio/virtio-rng.c
+++ b/hw/virtio/virtio-rng.c
@@ -109,11 +109,16 @@ static int virtio_rng_load(QEMUFile *f, void *opaque, int version_id)
 {
     VirtIORNG *vrng = opaque;
     VirtIODevice *vdev = VIRTIO_DEVICE(vrng);
+    int ret;
 
     if (version_id != 1) {
         return -EINVAL;
     }
-    virtio_load(vdev, f);
+
+    ret = virtio_load(vdev, f);
+    if (ret) {
+        return ret;
+    }
 
     /* We may have an element ready but couldn't process it due to a quota
      * limit.  Make sure to try again after live migration when the quota may

Comment 6 Miroslav Rezanina 2014-07-23 10:34:50 UTC
Fix included in qemu-kvm-0.12.1.2-2.431.el6

Comment 9 errata-xmlrpc 2014-10-14 07:01:19 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2014-1490.html