debian/patches/extra/0017-vhost-avoid-a-potential-use-of-an-uninitialized-vari.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= <clopez@suse.de>
   3 Date: Mon, 13 Feb 2023 09:57:47 +0100
   4 Subject: [PATCH] vhost: avoid a potential use of an uninitialized variable in
   5  vhost_svq_poll()
   6 MIME-Version: 1.0
   7 Content-Type: text/plain; charset=UTF-8
   8 Content-Transfer-Encoding: 8bit
   9
  10 In vhost_svq_poll(), if vhost_svq_get_buf() fails due to a device
  11 providing invalid descriptors, len is left uninitialized and returned
  12 to the caller, potentally leaking stack data or causing undefined
  13 behavior.
  14
  15 Fix this by initializing len to 0.
  16
  17 Found with GCC 13 and -fanalyzer (abridged):
  18
  19 ../hw/virtio/vhost-shadow-virtqueue.c: In function ‘vhost_svq_poll’:
  20 ../hw/virtio/vhost-shadow-virtqueue.c:538:12: warning: use of uninitialized value ‘len’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
  21   538 |     return len;
  22       |            ^~~
  23   ‘vhost_svq_poll’: events 1-4
  24     |
  25     |  522 | size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
  26     |      |        ^~~~~~~~~~~~~~
  27     |      |        |
  28     |      |        (1) entry to ‘vhost_svq_poll’
  29     |......
  30     |  525 |     uint32_t len;
  31     |      |              ~~~
  32     |      |              |
  33     |      |              (2) region created on stack here
  34     |      |              (3) capacity: 4 bytes
  35     |......
  36     |  528 |         if (vhost_svq_more_used(svq)) {
  37     |      |             ~
  38     |      |             |
  39     |      |             (4) inlined call to ‘vhost_svq_more_used’ from ‘vhost_svq_poll’
  40
  41     (...)
  42
  43     |  528 |         if (vhost_svq_more_used(svq)) {
  44     |      |            ^~~~~~~~~~~~~~~~~~~~~~~~~
  45     |      |            ||
  46     |      |            |(8) ...to here
  47     |      |            (7) following ‘true’ branch...
  48     |......
  49     |  537 |     vhost_svq_get_buf(svq, &len);
  50     |      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  51     |      |     |
  52     |      |     (9) calling ‘vhost_svq_get_buf’ from ‘vhost_svq_poll’
  53     |
  54     +--> ‘vhost_svq_get_buf’: events 10-11
  55            |
  56            |  416 | static VirtQueueElement *vhost_svq_get_buf(VhostShadowVirtqueue *svq,
  57            |      |                          ^~~~~~~~~~~~~~~~~
  58            |      |                          |
  59            |      |                          (10) entry to ‘vhost_svq_get_buf’
  60            |......
  61            |  423 |     if (!vhost_svq_more_used(svq)) {
  62            |      |          ~
  63            |      |          |
  64            |      |          (11) inlined call to ‘vhost_svq_more_used’ from ‘vhost_svq_get_buf’
  65            |
  66
  67            (...)
  68
  69            |
  70          ‘vhost_svq_get_buf’: event 14
  71            |
  72            |  423 |     if (!vhost_svq_more_used(svq)) {
  73            |      |        ^
  74            |      |        |
  75            |      |        (14) following ‘false’ branch...
  76            |
  77          ‘vhost_svq_get_buf’: event 15
  78            |
  79            |cc1:
  80            | (15): ...to here
  81            |
  82     <------+
  83     |
  84   ‘vhost_svq_poll’: events 16-17
  85     |
  86     |  537 |     vhost_svq_get_buf(svq, &len);
  87     |      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
  88     |      |     |
  89     |      |     (16) returning to ‘vhost_svq_poll’ from ‘vhost_svq_get_buf’
  90     |  538 |     return len;
  91     |      |            ~~~
  92     |      |            |
  93     |      |            (17) use of uninitialized value ‘len’ here
  94
  95 Note by  Laurent Vivier <lvivier@redhat.com>:
  96
  97     The return value is only used to detect an error:
  98
  99     vhost_svq_poll
 100         vhost_vdpa_net_cvq_add
 101             vhost_vdpa_net_load_cmd
 102                 vhost_vdpa_net_load_mac
 103                   -> a negative return is only used to detect error
 104                 vhost_vdpa_net_load_mq
 105                   -> a negative return is only used to detect error
 106             vhost_vdpa_net_handle_ctrl_avail
 107               -> a negative return is only used to detect error
 108
 109 Fixes: d368c0b052ad ("vhost: Do not depend on !NULL VirtQueueElement on vhost_svq_flush")
 110 Signed-off-by: Carlos López <clopez@suse.de>
 111 Message-Id: <20230213085747.19956-1-clopez@suse.de>
 112 Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
 113 Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
 114 (cherry-picked from commit e4dd39c699b7d63a06f686ec06ded8adbee989c1)
 115 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
 116 ---
 117  hw/virtio/vhost-shadow-virtqueue.c | 2 +-
 118  1 file changed, 1 insertion(+), 1 deletion(-)
 119
 120 diff --git a/hw/virtio/vhost-shadow-virtqueue.c b/hw/virtio/vhost-shadow-virtqueue.c
 121 index 5bd14cad96..a723073747 100644
 122 --- a/hw/virtio/vhost-shadow-virtqueue.c
 123 +++ b/hw/virtio/vhost-shadow-virtqueue.c
 124 @@ -522,7 +522,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
 125  size_t vhost_svq_poll(VhostShadowVirtqueue *svq)
 126  {
 127      int64_t start_us = g_get_monotonic_time();
 128 -    uint32_t len;
 129 +    uint32_t len = 0;
 130
 131      do {
 132          if (vhost_svq_more_used(svq)) {