| Message ID | 20220902085300.508078-2-alexander.ivanov@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parallels: Add duplication check, repair at open, fix bugs | expand |
On 9/2/22 10:52, Alexander Ivanov wrote: > The BDRVParallelsState structure contains data_end field that > is measured in sectors. > In parallels_open() initially this field is set by data_off field from > parallels image header. > > According to the parallels format documentation, > data_off field contains an offset, in sectors, from the start of the file > to the start of the data area. > For "WithoutFreeSpace" images: if data_off is zero, > the offset is calculated as the end of the BAT table > plus some padding to ensure sector size alignment. > > The parallels_open() function has code for handling zero value > in data_off, but in the result data_end contains the offset in bytes. > > Replace the alignment to sector size by division by sector size > and fix the comparision with s->header_size. > > Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> > --- > block/parallels.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index e6e8b9e369..7b4e997ebd 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -863,9 +863,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, > } > s->data_end = le32_to_cpu(ph.data_off); > if (s->data_end == 0) { > - s->data_end = ROUND_UP(bat_entry_off(s->bat_size), BDRV_SECTOR_SIZE); > + s->data_end = DIV_ROUND_UP(size, BDRV_SECTOR_SIZE); > } > - if (s->data_end < s->header_size) { > + if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) { > /* there is not enough unused space to fit to block align between BAT > and actual data. We can't avoid read-modify-write... */ > s->header_size = size; Reviewed-by: Denis V. Lunev <den@openvz.org> Sidenote: if the image is truncated for more than BAT size and there is no data inside the image, we would get memory corruption during BAT access. That should be addressed separately.
diff --git a/block/parallels.c b/block/parallels.c index e6e8b9e369..7b4e997ebd 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -863,9 +863,9 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, } s->data_end = le32_to_cpu(ph.data_off); if (s->data_end == 0) { - s->data_end = ROUND_UP(bat_entry_off(s->bat_size), BDRV_SECTOR_SIZE); + s->data_end = DIV_ROUND_UP(size, BDRV_SECTOR_SIZE); } - if (s->data_end < s->header_size) { + if (s->data_end < (s->header_size >> BDRV_SECTOR_BITS)) { /* there is not enough unused space to fit to block align between BAT and actual data. We can't avoid read-modify-write... */ s->header_size = size;
The BDRVParallelsState structure contains data_end field that is measured in sectors. In parallels_open() initially this field is set by data_off field from parallels image header. According to the parallels format documentation, data_off field contains an offset, in sectors, from the start of the file to the start of the data area. For "WithoutFreeSpace" images: if data_off is zero, the offset is calculated as the end of the BAT table plus some padding to ensure sector size alignment. The parallels_open() function has code for handling zero value in data_off, but in the result data_end contains the offset in bytes. Replace the alignment to sector size by division by sector size and fix the comparision with s->header_size. Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com> --- block/parallels.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)