On Fri, Mar 24, 2017 at 7:38 PM, David Hildenbrand [off-list ref] wrote:

On 23.03.2017 16:17, Arnd Bergmann wrote:

quoted

The latest gcc-7.0.1 snapshot reports a new warning:

virtio/virtio_balloon.c: In function 'update_balloon_stats':
virtio/virtio_balloon.c:258:26: error: 'events[2]' is used uninitialized in this function [-Werror=uninitialized]
virtio/virtio_balloon.c:260:26: error: 'events[3]' is used uninitialized in this function [-Werror=uninitialized]
virtio/virtio_balloon.c:261:56: error: 'events[18]' is used uninitialized in this function [-Werror=uninitialized]
virtio/virtio_balloon.c:262:56: error: 'events[17]' is used uninitialized in this function [-Werror=uninitialized]

This seems absolutely right, so we should add an extra check to
prevent copying uninitialized stack data into the statistics.
From all I can tell, this has been broken since the statistics code
was originally added in 2.6.34.

Fixes: 9564e138b1f6 ("virtio: Add memory statistics reporting to the balloon driver (V4)")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/virtio/virtio_balloon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 4e1191508228..cd5c54e2003d 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c

@@ -254,12 +254,14 @@ static void update_balloon_stats(struct virtio_balloon *vb)

      available = si_mem_available();

+#ifdef CONFIG_VM_EVENT_COUNTERS
      update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_IN,
                              pages_to_bytes(events[PSWPIN]));
      update_stat(vb, idx++, VIRTIO_BALLOON_S_SWAP_OUT,
                              pages_to_bytes(events[PSWPOUT]));
      update_stat(vb, idx++, VIRTIO_BALLOON_S_MAJFLT, events[PGMAJFAULT]);
      update_stat(vb, idx++, VIRTIO_BALLOON_S_MINFLT, events[PGFAULT]);
+#endif
      update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMFREE,
                              pages_to_bytes(i.freeram));
      update_stat(vb, idx++, VIRTIO_BALLOON_S_MEMTOT,

This will leave four uninitialized slots in vb->stats if
CONFIG_VM_EVENT_COUNTERS is not defined. update_balloon_stats should
have

  BUG_ON(idx < VIRTIO_BALLOON_S_NR);

at the end.

You need to make sure that vb->stats is smaller, either by using
something else than VIRTIO_BALLOON_S_NR for its size or something else
than sizeof(vb->stats) as the last argument to sg_init_one in this
file.

CC'ing Ladi

Thanks!

--

Thanks,

David