VirtualBox

Opened 7 years ago

Closed 7 years ago

#16311 closed defect (fixed)

VBoxManage modifyhd resize needs better parameter checking to prevent catastrophic data loss

Reported by: mpack Owned by:
Component: other Version: VirtualBox 5.1.10
Keywords: Cc:
Guest type: other Host type: other

Description

Please see the topic

https://forums.virtualbox.org/viewtopic.php?f=7&t=81093

for an example of the type of problem I'm going to discuss, however somebody makes this dumb mistake several times a year.

Scenario: they want to resize their disk, so they glance at the docs and don't notice that the resize size is in MB, therefore they provide a new size in bytes, often a very large new size. Obviously they don't make a backup either.

VBoxManage will go right ahead and attempt to create a logical drive of the chosen size * 1024*1024. Often this will fail for lack of space. Worse is when they realize their mistake and interrupt the work in mid stream. In some cases the block map alone is so huge that it won't fit on the host partition, but in all cases the most likely scenario is that their VM will not boot, and the disk contents have been scrambled.

It seems to me that VBoxManage should be doing some basic sanity checks on the resize size. This could be fancy but possibly incorrect (disk logical size can't be larger than host partition), or arbitrary - max size for resize cannot be larger than X terabytes, possibly with X depending on guest OS type. I'm sure you could dream up something sensible. But, being that it's an in-place function it definitely needs a filter for user stupidity.

Speaking of which: yeah, telling the user he should have made a backup is always good, but in that case one might wonder what the value of an "in place" method is.

No logs, since it's a design suggestion, not a crash. I set the affected version to 5.1.0, but really it's all versions since 4.0.0 to the latest 5.1.12 release.

Change History (9)

comment:1 by Frank Mehnert, 7 years ago

You have certainly a point. Normally the policy is that VBoxManage user should know what they are doing but in this specific case they don't have any other choice than using the VBoxManage interface. I think it would be appropriate to add a sanity check that the disk isn't resized to a size bigger than 1,000,000 times of the previous size. Of course only for the --resize parameter, not for the --resizebyte parameter.

We should also add the resize functionality to the GUI because it's functionality which is actually sometimes required.

comment:2 by Frank Mehnert, 7 years ago

Implemented the sanity check in diff:@64986:64989. This change will be part of the next 5.1.x maintenance release. The sanity check allows to resize a disk to a size with a maximum factor of 1000 if --resize is used.

comment:3 by Frank Mehnert, 7 years ago

Resolution: fixed
Status: newclosed

Fix is part of VBox 5.1.14.

comment:4 by mpack, 7 years ago

Resolution: fixed
Status: closedreopened

Reopening in order to report a bug in implementation. See the following discussion :-

https://forums.virtualbox.org/viewtopic.php?f=6&t=81512

It seems that when the container type is VHD then the sanity check code sees the old disk size as 0MB. This means that the size multiplier for the new size is always >1000 times 0MB, hence I guess it is now impossible to resize VHDs using --resize. As a workaround, --resizebytes still works.

Version 0, edited 7 years ago by mpack (next)

comment:5 by mpack, 7 years ago

Another instance of the 0MB problem reported here: https://forums.virtualbox.org/viewtopic.php?f=8&t=81524. definitely with a VDI this time.

In the new case the user made an additional error of asking for a new size which was smaller than the old size. I don't know if that's relevent, but it strikes me that the above sanity check already reports before and after sizes, so could be tweaked to give a more readable error for shrinkage attempts too.

comment:6 by White-Tiger, 7 years ago

A tool shouldn't limit the users capabilities by doing some assumptions... I suggest that VBoxManage asks the user whether to do the resize or not (after informing them that it's probably a bad idea) Moreover, it should handle a keyboard interrupt, power-loss and disk errors without breaking the entire disk. Recovery should be possible at all times (as much as possible. Power-loss is the worst case scenario and not easy to handle)

comment:7 by spuch, 7 years ago

I can confirm the Bug in VBox 5.1.14. Just tried to resize a vdi file from 20GB to 30GB and wondered if my given syntax is wrong.

C:\>"c:\Program Files\Oracle\VirtualBox\VBoxManage.exe" modifyhd "C:\Users\XXXXXX\VirtualBox VMs\Mageia Linux\Home.vdi" --resize 30720
VBoxManage.exe: error: Error: Attempt to resize the medium from 0.0 MB to 30720.0 MB. Use --resizeby
te if this is intended!
C:\>"c:\Program Files\Oracle\VirtualBox\VBoxManage.exe" showhdinfo "C:\Users\XXXXXX\VirtualBox VMs\Mageia Linux\Home.vdi"
UUID:           6c3e1942-XXXX-XXXX-XXXX-2ec758ed26df
Parent UUID:    base
State:          created
Type:           normal (base)
Location:       C:\Users\XXXXXX\VirtualBox VMs\Mageia Linux\Home.vdi
Storage format: VDI
Format variant: dynamic default
Capacity:       20480 MBytes
Size on disk:   20416 MBytes
Encryption:     disabled
In use by VMs:  Mageia Linux (UUID: 3bdd37ab-XXXX-XXXX-XXXX-022c26ff153e)

C:\>

comment:8 by Frank Mehnert, 7 years ago

Confirmed. This happens under certain conditions. A IMedium::RefreshState() was missing. Fixed in r65477.

comment:9 by Frank Mehnert, 7 years ago

Resolution: fixed
Status: reopenedclosed

Fix is part of VBox 5.1.16.

Note: See TracTickets for help on using tickets.

© 2023 Oracle
ContactPrivacy policyTerms of Use