Assertion failure through vring_split_desc_read

Assertion failure through vring_split_desc_read

[Alexander Bulekov]

Hello,
While fuzzing, I found an input that triggers an assertion failure
through virtio-rng -> vring_split_desc_read. Maybe this is related to:
Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu> 
Assertion failure through virtio_lduw_phys_cached

#8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
#9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
#10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
#11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
#12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
#13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
#14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
#15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
#16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
#17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
#18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9

I can reproduce it in a qemu 5.0 build using these qtest commands:
https://paste.debian.net/plain/1146089
(not including them here, as some are quite long)

wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace

Please let me know if I can provide any further info.
-Alex

Re: Assertion failure through vring_split_desc_read

[Laurent Vivier]

On 11/05/2020 05:51, Alexander Bulekov wrote:
> Hello,
> While fuzzing, I found an input that triggers an assertion failure
> through virtio-rng -> vring_split_desc_read. Maybe this is related to:
> Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu> 
> Assertion failure through virtio_lduw_phys_cached
> 
> #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
> #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
> #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
> #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
> #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
> #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
> #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
> #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
> #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
> #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
> #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
> 
> I can reproduce it in a qemu 5.0 build using these qtest commands:
> https://paste.debian.net/plain/1146089
> (not including them here, as some are quite long)
> 
> wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace

Nice work.

If I use directly "curl https://paste.debian.net/plain/1146089 |
qemu-system-i386 ..." I have only a warning:

    qemu-system-i386: Guest moved used index from 0 to 496

I've added some traces.

The assert is triggered because addr (0xffff0) >= cache->len (0x11d0).

addr is 2nd argument of:

    address_space_read_cached(cache, i * sizeof(VRingDesc),
                              desc, sizeof(VRingDesc));

and "i" appears to be "65535".

In virtqueue_split_read_next_desc(), the value is checked not to be
greater than max.

But max is 268345360...

"max" is provided by virtqueue_split_get_avail_bytes(), it's originally
vq->vring.num (255), but in the case of VRING_DESC_F_INDIRECT max is
updated to "desc.len / sizeof(VRingDesc)". desc.len is 4293525760.

It is set from max to cache->len by address_space_cache_init() but this
value seems to be truncated in the next iteration of the loop (where we
have the assert()).

I'm investigating why we have that.

Thanks,
Laurent

Re: Assertion failure through vring_split_desc_read

[Laurent Vivier]

On 12/05/2020 15:49, Laurent Vivier wrote:
> On 11/05/2020 05:51, Alexander Bulekov wrote:
>> Hello,
>> While fuzzing, I found an input that triggers an assertion failure
>> through virtio-rng -> vring_split_desc_read. Maybe this is related to:
>> Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu> 
>> Assertion failure through virtio_lduw_phys_cached
>>
>> #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
>> #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
>> #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
>> #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
>> #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
>> #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
>> #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
>> #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
>> #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
>> #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
>> #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
>>
>> I can reproduce it in a qemu 5.0 build using these qtest commands:
>> https://paste.debian.net/plain/1146089
>> (not including them here, as some are quite long)
>>
>> wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace
> 
> Nice work.
> 
> If I use directly "curl https://paste.debian.net/plain/1146089 |
> qemu-system-i386 ..." I have only a warning:
> 
>     qemu-system-i386: Guest moved used index from 0 to 496
> 
> I've added some traces.
> 
> The assert is triggered because addr (0xffff0) >= cache->len (0x11d0).
> 
> addr is 2nd argument of:
> 
>     address_space_read_cached(cache, i * sizeof(VRingDesc),
>                               desc, sizeof(VRingDesc));
> 
> and "i" appears to be "65535".
> 
> In virtqueue_split_read_next_desc(), the value is checked not to be
> greater than max.
> 
> But max is 268345360...
> 
> "max" is provided by virtqueue_split_get_avail_bytes(), it's originally
> vq->vring.num (255), but in the case of VRING_DESC_F_INDIRECT max is
> updated to "desc.len / sizeof(VRingDesc)". desc.len is 4293525760.
> 
> It is set from max to cache->len by address_space_cache_init() but this
> value seems to be truncated in the next iteration of the loop (where we
> have the assert()).
> 
> I'm investigating why we have that.

The problem is in virtqueue_split_get_avail_bytes().

On start, max is set to vq->vring.num (255), then in the
virtqueue_get_head() loop we read an indirect descriptor and max is set
to desc.len / sizeof(VRingDesc) (268345360). cache->len is
indirect_desc_cache.len, initialized from desc_len. So it is fine.

But after that desc is consumed by virtqueue_split_read_next_desc() and
then indirect_desc_cache is destroyed by address_space_cache_destroy(),
but max is not reset and used to check the next access with a new desc
that has a different size.

I think max should be reset when the cache is destroyed or set at the
begining of the loop.

Something like:

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b6c8ef5bc025..f6d90a99e5e4 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1001,7 +1001,6 @@ static void
virtqueue_split_get_avail_bytes(VirtQueue *vq,
     idx = vq->last_avail_idx;
     total_bufs = in_total = out_total = 0;

-    max = vq->vring.num;
     caches = vring_get_region_caches(vq);
     if (!caches) {
         goto err;
@@ -1013,6 +1012,7 @@ static void
virtqueue_split_get_avail_bytes(VirtQueue *vq,
         VRingDesc desc;
         unsigned int i;

+        max = vq->vring.num;
         num_bufs = total_bufs;

         if (!virtqueue_get_head(vq, idx++, &i)) {

Thanks,
Laurent

Re: Assertion failure through vring_split_desc_read

[John Snow]

On 5/10/20 11:51 PM, Alexander Bulekov wrote:
> Hello,
> While fuzzing, I found an input that triggers an assertion failure
> through virtio-rng -> vring_split_desc_read. Maybe this is related to:
> Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu> 
> Assertion failure through virtio_lduw_phys_cached
> 
> #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
> #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
> #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
> #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
> #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
> #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
> #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
> #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
> #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
> #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
> #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
> 
> I can reproduce it in a qemu 5.0 build using these qtest commands:
> https://paste.debian.net/plain/1146089
> (not including them here, as some are quite long)
> 
> wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace
> 
> Please let me know if I can provide any further info.
> -Alex
> 

Do you have a writeup somewhere of how you are approaching fuzzing and
how you've found this pile of bugs so far?

Might make for a good blog post.

--js

Re: Assertion failure through vring_split_desc_read

[Philippe Mathieu-Daudé]

On 5/14/20 1:24 AM, John Snow wrote:
> 
> 
> On 5/10/20 11:51 PM, Alexander Bulekov wrote:
>> Hello,
>> While fuzzing, I found an input that triggers an assertion failure
>> through virtio-rng -> vring_split_desc_read. Maybe this is related to:
>> Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu>
>> Assertion failure through virtio_lduw_phys_cached
>>
>> #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
>> #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
>> #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
>> #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
>> #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
>> #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
>> #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
>> #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
>> #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
>> #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
>> #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
>>
>> I can reproduce it in a qemu 5.0 build using these qtest commands:
>> https://paste.debian.net/plain/1146089
>> (not including them here, as some are quite long)
>>
>> wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace
>>
>> Please let me know if I can provide any further info.
>> -Alex
>>
> 
> Do you have a writeup somewhere of how you are approaching fuzzing and
> how you've found this pile of bugs so far?

There is docs/devel/fuzzing.txt:

https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/fuzzing.txt;hb=v5.0.0

> 
> Might make for a good blog post.

Good idea!

> 
> --js
> 
> 

Re: Assertion failure through vring_split_desc_read

[Alexander Bulekov]

On 200514 1012, Philippe Mathieu-Daudé wrote:
> On 5/14/20 1:24 AM, John Snow wrote:
> > 
> > 
> > On 5/10/20 11:51 PM, Alexander Bulekov wrote:
> > > Hello,
> > > While fuzzing, I found an input that triggers an assertion failure
> > > through virtio-rng -> vring_split_desc_read. Maybe this is related to:
> > > Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu>
> > > Assertion failure through virtio_lduw_phys_cached
> > > 
> > > #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
> > > #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
> > > #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
> > > #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
> > > #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
> > > #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
> > > #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
> > > #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
> > > #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
> > > #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
> > > #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
> > > 
> > > I can reproduce it in a qemu 5.0 build using these qtest commands:
> > > https://paste.debian.net/plain/1146089
> > > (not including them here, as some are quite long)
> > > 
> > > wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace
> > > 
> > > Please let me know if I can provide any further info.
> > > -Alex
> > > 
> > 
> > Do you have a writeup somewhere of how you are approaching fuzzing and
> > how you've found this pile of bugs so far?
> 
> There is docs/devel/fuzzing.txt:
> 
> https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/fuzzing.txt;hb=v5.0.0
> 
> > 
> > Might make for a good blog post.

I am working on a patchset for the particular fuzzer I used to find
these bugs. With that, I'll also update docs/devel/fuzzing.txt.

> 
> Good idea!

Yes I agree :)

> 
> > 
> > --js
> > 
> > 
> 

Re: Assertion failure through vring_split_desc_read

[John Snow]

On 5/14/20 9:50 AM, Alexander Bulekov wrote:
> On 200514 1012, Philippe Mathieu-Daudé wrote:
>> On 5/14/20 1:24 AM, John Snow wrote:
>>>
>>>
>>> On 5/10/20 11:51 PM, Alexander Bulekov wrote:
>>>> Hello,
>>>> While fuzzing, I found an input that triggers an assertion failure
>>>> through virtio-rng -> vring_split_desc_read. Maybe this is related to:
>>>> Message-ID: <20200511033001.dzvtbdhl3oz5pgiy@mozz.bu.edu>
>>>> Assertion failure through virtio_lduw_phys_cached
>>>>
>>>> #8 0x7fe6a9acf091 in __assert_fail /build/glibc-GwnBeO/glibc-2.30/assert/assert.c:101:3
>>>> #9 0x564cbe7d96fd in address_space_read_cached include/exec/memory.h:2423:5
>>>> #10 0x564cbe7e79c5 in vring_split_desc_read hw/virtio/virtio.c:236:5
>>>> #11 0x564cbe7e84ce in virtqueue_split_read_next_desc hw/virtio/virtio.c:929:5
>>>> #12 0x564cbe78f86b in virtqueue_split_get_avail_bytes hw/virtio/virtio.c:1009:18
>>>> #13 0x564cbe78ab22 in virtqueue_get_avail_bytes hw/virtio/virtio.c:1208:9
>>>> #14 0x564cc08aade1 in get_request_size hw/virtio/virtio-rng.c:40:5
>>>> #15 0x564cc08aa20b in virtio_rng_process hw/virtio/virtio-rng.c:115:12
>>>> #16 0x564cc08a8c48 in virtio_rng_set_status hw/virtio/virtio-rng.c:172:5
>>>> #17 0x564cbe7a50be in virtio_set_status hw/virtio/virtio.c:1876:9
>>>> #18 0x564cc08d1b8f in virtio_pci_common_write hw/virtio/virtio-pci.c:1245:9
>>>>
>>>> I can reproduce it in a qemu 5.0 build using these qtest commands:
>>>> https://paste.debian.net/plain/1146089
>>>> (not including them here, as some are quite long)
>>>>
>>>> wget https://paste.debian.net/plain/1146089 -O qtest-trace; ~/Development/qemu/build/i386-softmmu/qemu-system-i386 -M pc-q35-5.0  -device virtio-rng-pci,addr=04.0 -display none -nodefaults -nographic -qtest stdio < qtest-trace
>>>>
>>>> Please let me know if I can provide any further info.
>>>> -Alex
>>>>
>>>
>>> Do you have a writeup somewhere of how you are approaching fuzzing and
>>> how you've found this pile of bugs so far?
>>
>> There is docs/devel/fuzzing.txt:
>>
>> https://git.qemu.org/?p=qemu.git;a=blob;f=docs/devel/fuzzing.txt;hb=v5.0.0
>>
>>>
>>> Might make for a good blog post.
> 
> I am working on a patchset for the particular fuzzer I used to find
> these bugs. With that, I'll also update docs/devel/fuzzing.txt.
> 
>>
>> Good idea!
> 
> Yes I agree :)
> 

Awesome, I look forward to it. Thanks for the reports and the bugs filed
on LP.

--js