openstack/neutron
Code Issues Proposed changes

Fix potential race condition in privileged ip_lib module

Browse Source 
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
This commit is contained in:
Sławek Kapłoński 2018-04-12 13:02:56 +02:00
parent 2f3fb39fa2
commit 56324c12aa
2 changed files with 58 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
neutron/privileged/agent/linux/ip_lib.py
View File

@ -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)

33
neutron/tests/unit/privileged/agent/linux/test_ip_lib.py
View File

@ -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(