[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 2/6] qcow2: simplify qcow2_cache_pu
From: |
Alberto Garcia |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 2/6] qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty() |
Date: |
Wed, 06 May 2015 17:32:47 +0200 |
User-agent: |
Notmuch/0.13.2 (http://notmuchmail.org) Emacs/23.2.1 (i486-pc-linux-gnu) |
On Wed 06 May 2015 05:00:50 PM CEST, Stefan Hajnoczi wrote:
>> >> int qcow2_cache_put(BlockDriverState *bs, Qcow2Cache *c, void **table)
>> >> {
>> >> - int i;
>> >> + int i = (*table - c->table_array) / c->table_size;
>> >>
>> >> - for (i = 0; i < c->size; i++) {
>> >> - if (table_addr(c, i) == *table) {
>> >> - goto found;
>> >> - }
>> >> + if (c->entries[i].offset == 0) {
>> >> + return -ENOENT;
>> >> }
>> >> - return -ENOENT;
>> >>
>> >> -found:
>> >> c->entries[i].ref--;
>> >> *table = NULL;
>> >>
>> >
>> > When is this function called with a bogus table pointer?
>>
>> I also could not image any such scenario, but I decided to be
>> conservative and keep the error handling code. I'll double check all
>> places where it's used and remove the relevant code.
>
> The reason why I'd think this is worth looking into is:
>
> The new qcow2_cache_put() code indexes outside the array bounds if
>table is a bogus value. The old code would return -ENOENT. So I am a
>little nervous about the change, although I suspect the function is
>never called with invalid inputs at all.
I checked again all callers of qcow2_cache_put() and I didn't notice
anything unusual.
In some cases it even goes after qcow2_cache_entry_mark_dirty(), and
that one directly aborts if the table pointer is bogus.
Berto