Re: [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries = 0x40000000
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 0x1, 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. pgpUgJtzhXuNR.pgp Description: PGP signature
Re: [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries = 0x40000000
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 0x1, 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 pgpwMzEoRfQNy.pgp Description: PGP signature
Re: [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries = 0x40000000
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 0x1, 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. pgpBKUy3yog9o.pgp Description: PGP signature
Re: [Qemu-block] [PATCH 1/2] block: vpc - prevent overflow if max_table_entries = 0x40000000
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? pgpwyiuZ69mPx.pgp Description: PGP signature
