linux-kernel - Re: [PATCH] virtio_balloon: prevent uninitialized variable use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Mar 2017 21:11:25 +0100
From:   Ladi Prosek <lprosek@...hat.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Jason Wang <jasowang@...hat.com>,
        Yisheng Xie <xieyisheng1@...wei.com>,
        Konstantin Neumoin <kneumoin@...tuozzo.com>,
        linux-kernel@...r.kernel.org,
        virtualization@...ts.linux-foundation.org,
        Minchan Kim <minchan@...nel.org>,
        "Denis V. Lunev" <den@...nvz.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH] virtio_balloon: prevent uninitialized variable use

On Fri, Mar 24, 2017 at 7:38 PM, David Hildenbrand <david@...hat.com> wrote:
> On 23.03.2017 16:17, Arnd Bergmann wrote:
>> 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@...db.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