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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

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?

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]