From 56324c12aa58ff9cf46582611b071f3abb743ddf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C5=82awek=20Kap=C5=82o=C5=84ski?= Date: Thu, 12 Apr 2018 13:02:56 +0200 Subject: [PATCH] Fix potential race condition in privileged ip_lib module Functions like _run_iproute_{link,addr,neigh} are not atomic and work in two steps. First it tries to get device index and in second step calls specified command for this device. It might happen sometimes that device exists during first of those steps but not exists during second step. Such case causes raising pyroute2.NetlinkError exception which isn't properly handled in Neutron code which uses ip_lib module. This patch fixes it by catching pyroute2.NetlinkError exception and raising NetworkInterfaceNotFound. This is subclass of RuntimeError and all callers of ip_lib can handle it properly is needed. Change-Id: I568ef183466f5ff2f2c30ed74a7dc52db41ba577 Closes-Bug: #1763329 --- neutron/privileged/agent/linux/ip_lib.py | 30 ++++++++++++++--- .../privileged/agent/linux/test_ip_lib.py | 33 +++++++++++++++++++ 2 files changed, 58 insertions(+), 5 deletions(-) diff --git a/neutron/privileged/agent/linux/ip_lib.py b/neutron/privileged/agent/linux/ip_lib.py index 8c9e10b4e6f..14157f277c8 100644 --- a/neutron/privileged/agent/linux/ip_lib.py +++ b/neutron/privileged/agent/linux/ip_lib.py @@ -48,7 +48,18 @@ class NetworkNamespaceNotFound(RuntimeError): class NetworkInterfaceNotFound(RuntimeError): - pass + message = _("Network interface %(device)s not found in namespace " + "%(namespace)s.") + + def __init__(self, message=None, device=None, namespace=None): + # NOTE(slaweq): 'message' can be passed as an optional argument + # because of how privsep daemon works. If exception is raised in + # function called by privsep daemon, it will then try to reraise it + # and will call it always with passing only message from originally + # raised exception. + message = message or self.message % { + 'device': device, 'namespace': namespace} + super(NetworkInterfaceNotFound, self).__init__(message) class IpAddressAlreadyExists(RuntimeError): @@ -110,10 +121,7 @@ def _get_link_id(device, namespace): with _get_iproute(namespace) as ip: return ip.link_lookup(ifname=device)[0] except IndexError: - msg = _("Network interface %(device)s not found in namespace " - "%(namespace)s.") % {'device': device, - 'namespace': namespace} - raise NetworkInterfaceNotFound(msg) + raise NetworkInterfaceNotFound(device=device, namespace=namespace) def _run_iproute_link(command, device, namespace=None, **kwargs): @@ -121,6 +129,10 @@ def _run_iproute_link(command, device, namespace=None, **kwargs): with _get_iproute(namespace) as ip: idx = _get_link_id(device, namespace) return ip.link(command, index=idx, **kwargs) + except NetlinkError as e: + if e.code == errno.ENODEV: + raise NetworkInterfaceNotFound(device=device, namespace=namespace) + raise except OSError as e: if e.errno == errno.ENOENT: raise NetworkNamespaceNotFound(netns_name=namespace) @@ -132,6 +144,10 @@ def _run_iproute_neigh(command, device, namespace, **kwargs): with _get_iproute(namespace) as ip: idx = _get_link_id(device, namespace) return ip.neigh(command, ifindex=idx, **kwargs) + except NetlinkError as e: + if e.code == errno.ENODEV: + raise NetworkInterfaceNotFound(device=device, namespace=namespace) + raise except OSError as e: if e.errno == errno.ENOENT: raise NetworkNamespaceNotFound(netns_name=namespace) @@ -143,6 +159,10 @@ def _run_iproute_addr(command, device, namespace, **kwargs): with _get_iproute(namespace) as ip: idx = _get_link_id(device, namespace) return ip.addr(command, index=idx, **kwargs) + except NetlinkError as e: + if e.code == errno.ENODEV: + raise NetworkInterfaceNotFound(device=device, namespace=namespace) + raise except OSError as e: if e.errno == errno.ENOENT: raise NetworkNamespaceNotFound(netns_name=namespace) diff --git a/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py b/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py index 1024bf59273..6c85128903b 100644 --- a/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py +++ b/neutron/tests/unit/privileged/agent/linux/test_ip_lib.py @@ -51,6 +51,17 @@ class IpLibTestCase(base.BaseTestCase): priv_lib._run_iproute_link, "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_link_interface_removed_during_call(self): + with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: + ip_mock = iproute_mock() + ip_mock.__enter__().link_lookup.return_value = [2] + ip_mock.__enter__().link.side_effect = pyroute2.NetlinkError( + code=errno.ENODEV) + self.assertRaises( + priv_lib.NetworkInterfaceNotFound, + priv_lib._run_iproute_link, + "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_link_namespace_not_exists(self): with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: iproute_mock.side_effect = OSError( @@ -99,6 +110,17 @@ class IpLibTestCase(base.BaseTestCase): priv_lib._run_iproute_neigh, "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_neigh_interface_removed_during_call(self): + with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: + ip_mock = iproute_mock() + ip_mock.__enter__().link_lookup.return_value = [2] + ip_mock.__enter__().neigh.side_effect = pyroute2.NetlinkError( + code=errno.ENODEV) + self.assertRaises( + priv_lib.NetworkInterfaceNotFound, + priv_lib._run_iproute_neigh, + "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_neigh_namespace_not_exists(self): with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: iproute_mock.side_effect = OSError( @@ -147,6 +169,17 @@ class IpLibTestCase(base.BaseTestCase): priv_lib._run_iproute_addr, "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_addr_interface_removed_during_call(self): + with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: + ip_mock = iproute_mock() + ip_mock.__enter__().link_lookup.return_value = [2] + ip_mock.__enter__().addr.side_effect = pyroute2.NetlinkError( + code=errno.ENODEV) + self.assertRaises( + priv_lib.NetworkInterfaceNotFound, + priv_lib._run_iproute_addr, + "test_cmd", "eth0", None, test_param="test_value") + def test_run_iproute_addr_namespace_not_exists(self): with mock.patch.object(pyroute2, "IPRoute") as iproute_mock: iproute_mock.side_effect = OSError(