openstack/nova
Code Issues Proposed changes

Make disk.extend() pass format to qemu-img

Browse Source 
This fixes an instance of us passing a disk image to qemu-img for
resize where we don't constrain the format. As has previously been
identified, it is never safe to do that when the image itself is not
trusted. In this case, an instance with a previously-raw disk image
being used by imagebackend.Flat is susceptible to the user writing a
qcow2 (or other) header to their disk causing the unconstrained
qemu-img resize operation to interpret it as a qcow2 file.

Since Flat maintains the intended disk format in the disk.info file,
and since we would have safety-checked images we got from glance,
we should be able to trust the image.format specifier, which comes
from driver_format in imagebackend, which is read from disk.info.
Since only raw or qcow2 files should be resized anyway, we can further
constrain it to those.

Notes:
 1. qemu-img refuses to resize some types of VMDK files, but it may
    be able to resize others (there are many subformats). Technically,
    Flat will allow running an instance directly from a VMDK file,
    and so this change _could_ be limiting existing "unintentionally
    works" behavior.
 2. This assumes that disk.info is correct, present, etc. The code to
    handle disk.info will regenerate the file if it's missing or
    unreadable by probing the image without a safety check, which
    would be unsafe. However, that is a much more sophisticated attack,
    requiring either access to the system to delete the file or an
    errant operator action in the first place.

Change-Id: I07cbe90b7a7a0a416ef13fbc3a1b7e2272c90951
Closes-Bug: #2137507
Signed-off-by: Dan Smith <dansmith@redhat.com>
This commit is contained in:
Dan Smith
2026-02-17 06:35:35 -08:00
parent 4b90fdf9af
commit 3eba22ff09
2 changed files with 46 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

35
nova/tests/unit/virt/disk/test_api.py
View File

@@ -19,6 +19,7 @@ from unittest import mock
from oslo_concurrency import processutils
from oslo_utils import units
from nova import exception
from nova import test
from nova.virt.disk import api
from nova.virt.disk.mount import api as mount
@@ -128,7 +129,7 @@ class APITestCase(test.NoDBTestCase):
mock_can_resize.assert_called_once_with(imgfile, imgsize)
mock_exec.assert_called_once_with('qemu-img', 'resize',
imgfile, imgsize)
'-f', 'qcow2', imgfile, imgsize)
mock_extendable.assert_called_once_with(image)
mock_inst.assert_called_once_with(image, None, None)
mock_resize.assert_called_once_with(mounter.device,
@@ -154,8 +155,8 @@ class APITestCase(test.NoDBTestCase):
api.extend(image, imgsize)
mock_can_resize_image.assert_called_once_with(imgfile, imgsize)
mock_execute.assert_called_once_with('qemu-img', 'resize', imgfile,
imgsize)
mock_execute.assert_called_once_with('qemu-img', 'resize', '-f',
'qcow2', imgfile, imgsize)
self.assertFalse(mock_extendable.called)
@mock.patch.object(api, 'can_resize_image', autospec=True,
@@ -186,8 +187,34 @@ class APITestCase(test.NoDBTestCase):
api.extend(image, imgsize)
mock_exec.assert_has_calls(
[mock.call('qemu-img', 'resize', imgfile, imgsize),
[mock.call('qemu-img', 'resize', '-f', 'raw', imgfile, imgsize),
mock.call('e2label', image.path)])
mock_resize.assert_called_once_with(imgfile, run_as_root=False,
check_exit_code=[0])
mock_can_resize.assert_called_once_with(imgfile, imgsize)
@mock.patch.object(api, 'can_resize_image', autospec=True,
return_value=True)
@mock.patch.object(api, 'resize2fs', autospec=True)
@mock.patch('oslo_concurrency.processutils.execute', autospec=True)
def test_extend_vmdk_failure(self, mock_exec, mock_resize,
mock_can_resize):
imgfile = tempfile.NamedTemporaryFile()
self.addCleanup(imgfile.close)
imgsize = 10
# NOTE(danms): There is no image.model.FORMAT_VMDK, but since the
# code initializes this directly from Image.disk_format without using
# the constant (tsk), this can actually happen at runtime.
self.assertRaises(exception.InvalidImageFormat,
imgmodel.LocalFileImage, imgfile, 'vmdk')
# Patch ALL_FORMATS to include vmdk as if it got added at some point
with mock.patch('nova.virt.image.model.ALL_FORMATS',
new=['vmdk']):
image = imgmodel.LocalFileImage(imgfile, 'vmdk')
# Make sure that we still don't call qemu-img resize on the image
self.assertRaises(exception.InvalidDiskFormat,
api.extend, image, imgsize)
mock_exec.assert_not_called()

16
nova/virt/disk/api.py
View File

@@ -123,7 +123,21 @@ def extend(image, size):
nova.privsep.libvirt.ploop_resize(image.path, size)
return
processutils.execute('qemu-img', 'resize', image.path, size)
# NOTE(danms): We should not call qemu-img without a format, and
# only qcow2 and raw are supported. So check which one we're being
# told this is supposed to be and pass that to qemu-img. Also note
# that we need to pass the qemu format string to this command, which
# may or may not be the same as the FORMAT_* constant, so be
# explicit here.
if image.format == imgmodel.FORMAT_RAW:
format = 'raw'
elif image.format == imgmodel.FORMAT_QCOW2:
format = 'qcow2'
else:
LOG.warning('Attempting to resize image %s with format %s, '
'which is not supported', image.path, image.format)
raise exception.InvalidDiskFormat(disk_format=image.format)
processutils.execute('qemu-img', 'resize', '-f', format, image.path, size)
if (image.format != imgmodel.FORMAT_RAW and
not CONF.resize_fs_using_block_device):