openstack/nova
Code Issues Proposed changes

virt: set address space & CPU time limits when running qemu-img

Browse Source 
This uses the new 'prlimit' parameter for oslo.concurrency execute
method, to set an address space limit of 1GB and CPU time limit
of 2 seconds, when running qemu-img.

This is a re-implementation of the previously reverted commit

commit da217205f5
Author: Tristan Cacqueray <tdecacqu@redhat.com>
Date:   Wed Aug 5 17:17:04 2015 +0000

    virt: Use preexec_fn to ulimit qemu-img info call

NOTE (kchamart) [stable/liberty]: Add a check for the presence of
'ProcessLimits' attribute (which is only present in
oslo.concurrency>=2.6.1; and a conditional check for 'prlimit' parameter
in qemu_img_info() method.

Upstream discussion[1][2] that led to merging this patch to
stable/liberty branch.

[1] http://lists.openstack.org/pipermail/openstack-dev/2016-September/104091.html
[2] http://lists.openstack.org/pipermail/openstack-dev/2016-September/104303.html

Closes-Bug: #1449062
Change-Id: I135b5242af1bfdcb0ea09a6fcda21fc03a6fbe7d
(cherry picked from commit 068d851561)
This commit is contained in:
Daniel P. Berrange
2016-04-18 16:32:19 +00:00
committed by Kashyap Chamarthy
parent c55aacfbac
commit 6bc37dccec
4 changed files with 46 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
nova/tests/unit/virt/libvirt/test_driver.py
View File

@@ -85,6 +85,7 @@ from nova.virt import fake
from nova.virt import firewall as base_firewall from nova.virt import firewall as base_firewall
from nova.virt import hardware from nova.virt import hardware
from nova.virt.image import model as imgmodel from nova.virt.image import model as imgmodel
from nova.virt import images
from nova.virt.libvirt import blockinfo from nova.virt.libvirt import blockinfo
from nova.virt.libvirt import config as vconfig from nova.virt.libvirt import config as vconfig
from nova.virt.libvirt import driver as libvirt_driver from nova.virt.libvirt import driver as libvirt_driver
@@ -7704,7 +7705,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
self.mox.StubOutWithMock(utils, "execute") self.mox.StubOutWithMock(utils, "execute")
utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info',
'/test/disk.local').AndReturn((ret, '')) '/test/disk.local', prlimit=images.QEMU_IMG_LIMITS,
).AndReturn((ret, ''))
self.mox.ReplayAll() self.mox.ReplayAll()
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@@ -7812,7 +7814,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
self.mox.StubOutWithMock(utils, "execute") self.mox.StubOutWithMock(utils, "execute")
utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', utils.execute('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info',
'/test/disk.local').AndReturn((ret, '')) '/test/disk.local', prlimit=images.QEMU_IMG_LIMITS,
).AndReturn((ret, ''))
self.mox.ReplayAll() self.mox.ReplayAll()
conn_info = {'driver_volume_type': 'fake'} conn_info = {'driver_volume_type': 'fake'}

27
nova/tests/unit/virt/libvirt/test_utils.py
View File

@@ -91,7 +91,8 @@ disk size: 96K
mock_execute.return_value = (output, '') mock_execute.return_value = (output, '')
d_backing = libvirt_utils.get_disk_backing_file(path) d_backing = libvirt_utils.get_disk_backing_file(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertIsNone(d_backing) self.assertIsNone(d_backing)
@@ -99,7 +100,8 @@ disk size: 96K
d_size = libvirt_utils.get_disk_size(path) d_size = libvirt_utils.get_disk_size(path)
self.assertEqual(expected_size, d_size) self.assertEqual(expected_size, d_size)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
@mock.patch('os.path.exists', return_value=True) @mock.patch('os.path.exists', return_value=True)
def test_disk_size(self, mock_exists): def test_disk_size(self, mock_exists):
@@ -145,7 +147,8 @@ blah BLAH: bb
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
image_info = images.qemu_img_info(path) image_info = images.qemu_img_info(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertEqual('disk.config', image_info.image) self.assertEqual('disk.config', image_info.image)
self.assertEqual('raw', image_info.file_format) self.assertEqual('raw', image_info.file_format)
@@ -167,7 +170,8 @@ backing file: /var/lib/nova/a328c7998805951a_2
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
image_info = images.qemu_img_info(path) image_info = images.qemu_img_info(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertEqual('disk.config', image_info.image) self.assertEqual('disk.config', image_info.image)
self.assertEqual('qcow2', image_info.file_format) self.assertEqual('qcow2', image_info.file_format)
@@ -195,7 +199,8 @@ backing file: /var/lib/nova/a328c7998805951a_2 (actual path: /b/3a988059e51a_2)
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
image_info = images.qemu_img_info(path) image_info = images.qemu_img_info(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertEqual('disk.config', image_info.image) self.assertEqual('disk.config', image_info.image)
self.assertEqual('raw', image_info.file_format) self.assertEqual('raw', image_info.file_format)
@@ -223,7 +228,8 @@ junk stuff: bbb
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
image_info = images.qemu_img_info(path) image_info = images.qemu_img_info(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertEqual('disk.config', image_info.image) self.assertEqual('disk.config', image_info.image)
self.assertEqual('raw', image_info.file_format) self.assertEqual('raw', image_info.file_format)
@@ -247,7 +253,8 @@ ID TAG VM SIZE DATE VM CLOCK
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
image_info = images.qemu_img_info(path) image_info = images.qemu_img_info(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
self.assertEqual('disk.config', image_info.image) self.assertEqual('disk.config', image_info.image)
self.assertEqual('raw', image_info.file_format) self.assertEqual('raw', image_info.file_format)
@@ -283,7 +290,8 @@ ID TAG VM SIZE DATE VM CLOCK
mock_execute.return_value = ('stdout', None) mock_execute.return_value = ('stdout', None)
libvirt_utils.create_cow_image('/some/path', '/the/new/cow') libvirt_utils.create_cow_image('/some/path', '/the/new/cow')
expected_args = [(('env', 'LC_ALL=C', 'LANG=C', expected_args = [(('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', '/some/path'),), 'qemu-img', 'info', '/some/path'),
{'prlimit': images.QEMU_IMG_LIMITS}),
(('qemu-img', 'create', '-f', 'qcow2', (('qemu-img', 'create', '-f', 'qcow2',
'-o', 'backing_file=/some/path', '-o', 'backing_file=/some/path',
'/the/new/cow'),)] '/the/new/cow'),)]
@@ -369,7 +377,8 @@ disk size: 4.4M
mock_execute.return_value = (example_output, '') mock_execute.return_value = (example_output, '')
self.assertEqual(4592640, disk.get_disk_size('/some/path')) self.assertEqual(4592640, disk.get_disk_size('/some/path'))
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C', mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path) 'qemu-img', 'info', path,
prlimit=images.QEMU_IMG_LIMITS)
mock_exists.assert_called_once_with(path) mock_exists.assert_called_once_with(path)
def test_copy_image(self): def test_copy_image(self):

16
nova/virt/images.py
View File

@@ -25,6 +25,7 @@ from oslo_concurrency import processutils
from oslo_config import cfg from oslo_config import cfg
from oslo_log import log as logging from oslo_log import log as logging
from oslo_utils import fileutils from oslo_utils import fileutils
from oslo_utils import units
from nova import exception from nova import exception
from nova.i18n import _, _LE from nova.i18n import _, _LE
@@ -43,6 +44,16 @@ image_opts = [
CONF = cfg.CONF CONF = cfg.CONF
CONF.register_opts(image_opts) CONF.register_opts(image_opts)
IMAGE_API = image.API() IMAGE_API = image.API()
QEMU_IMG_LIMITS = None
try:
QEMU_IMG_LIMITS = processutils.ProcessLimits(
cpu_time=2,
address_space=1 * units.Gi)
except Exception:
LOG.error(_LE('Please upgrade to oslo.concurrency version '
'2.6.1 -- this version has fixes for the '
'vulnerability CVE-2015-5162.'))
def qemu_img_info(path, format=None): def qemu_img_info(path, format=None):
@@ -61,7 +72,10 @@ def qemu_img_info(path, format=None):
if format is not None: if format is not None:
cmd = cmd + ('-f', format) cmd = cmd + ('-f', format)
try: try:
out, err = utils.execute(*cmd) if QEMU_IMG_LIMITS is not None:
out, err = utils.execute(*cmd, prlimit=QEMU_IMG_LIMITS)
else:
out, err = utils.execute(*cmd)
except processutils.ProcessExecutionError as exp: except processutils.ProcessExecutionError as exp:
msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") % msg = (_("qemu-img failed to execute on %(path)s : %(exp)s") %
{'path': path, 'exp': exp}) {'path': path, 'exp': exp})

8
releasenotes/notes/apply-limits-to-qemu-img-8813f7a333ebdf69.yaml Normal file
View File

@@ -0,0 +1,8 @@
---
security:
- The qemu-img tool now has resource limits applied
which prevent it from using more than 1GB of address
space or more than 2 seconds of CPU time. This provides
protection against denial of service attacks from
maliciously crafted or corrupted disk images.
oslo.concurrency>=2.6.1 is required for this fix.