Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries >= 0x40000000

[Stefan Hajnoczi]

On Wed, Jul 08, 2015 at 01:55:34PM +0200, Kevin Wolf wrote:
> Am 26.06.2015 um 11:57 hat Stefan Hajnoczi geschrieben:
> > On Thu, Jun 25, 2015 at 11:05:20AM -0400, Jeff Cody wrote:
> > > On Thu, Jun 25, 2015 at 03:28:35PM +0100, Stefan Hajnoczi wrote:
> > > > On Wed, Jun 24, 2015 at 03:54:27PM -0400, Jeff Cody wrote:
> > > > > @@ -269,7 +270,9 @@ static int vpc_open(BlockDriverState *bs, QDict 
> > > > > *options, int flags,
> > > > >              goto fail;
> > > > >          }
> > > > >  
> > > > > -        s->pagetable = qemu_try_blockalign(bs->file, 
> > > > > s->max_table_entries * 4);
> > > > > +        pagetable_size = (size_t) s->max_table_entries * 4;
> > > > > +
> > > > > +        s->pagetable = qemu_try_blockalign(bs->file, pagetable_size);
> > > > 
> > > > On 32-bit hosts size_t is 32-bit so the overflow hasn't been solved.
> > > > 
> > > > Does it make sense to impose a limit on pagetable_size?
> > > 
> > > Good point.  Yes, it does.
> > > 
> > > The VHD spec says that the "Max Table Entries" field should be equal
> > > to the disk size / block size.  I don't know if there are images out
> > > there that treat that as ">= disk size / block size" rather than "==",
> > > however.  But if we assume max size of 2TB for a VHD disk, and a
> > > minimal block size of 512 bytes, that would give us a
> > > max_table_entries of 0x100000000, which exceeds 32 bits by itself.
> > > 
> > > For pagetable_size to fit in a 32-bit, that means to support 2TB on a
> > > 32-bit host in the current implementation, the minimum block size is
> > > 4096.
> > > 
> > > We could check during open / create that 
> > > (disk_size / block_size) * 4 < SIZE_MAX, and refuse to open if this is
> > > not true (and also validate max_table_entries to fit in the same).
> > 
> > Sounds good.
> 
> Jeff, I couldn't find a v2 anywhere. Are you still planning to send it?

Jeff is on vacation.  I think he's back next week.

pgp_JemfcM4RO.pgp
Description: PGP signature