From 992646e49b4b4d96f3258dc154d6f00a43d18d01 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Tue, 17 Feb 2026 06:37:57 -0800 Subject: [PATCH] Make disk.extend() pass format to qemu-img 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 (cherry picked from commit 3eba22ff09c81a61750fbb4882e5f1f01a20fdf5) (cherry picked from commit f448173e3c531f3b298ed2f6f02ff9b47981fbc1) Signed-off-by: Dan Smith --- nova/tests/unit/virt/disk/test_api.py | 35 ++++++++++++++++++++++++--- nova/virt/disk/api.py | 16 +++++++++++- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/nova/tests/unit/virt/disk/test_api.py b/nova/tests/unit/virt/disk/test_api.py index 135558e14553..47842276555e 100644 --- a/nova/tests/unit/virt/disk/test_api.py +++ b/nova/tests/unit/virt/disk/test_api.py @@ -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() diff --git a/nova/virt/disk/api.py b/nova/virt/disk/api.py index 96e9d4a2da3c..8d643635f227 100644 --- a/nova/virt/disk/api.py +++ b/nova/virt/disk/api.py @@ -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):