Re: [Qemu-devel] [PATCH 2/6] qcow2: simplify qcow2_cache_put() and qcow2_cache_entry_mark_dirty()

[Stefan Hajnoczi]

On Tue, May 05, 2015 at 03:06:52PM +0200, Alberto Garcia wrote:
> On Fri 01 May 2015 04:31:52 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.

Stefan

pgpB4yqIrzSkA.pgp
Description: PGP signature