From e10343b7e554313a077527a5c57b3947663c24d7 Mon Sep 17 00:00:00 2001 From: Olivier Gayot Date: Wed, 6 Sep 2023 17:06:35 +0200 Subject: [PATCH] network: fix crash when Wi-Fi or eth interface gets removed from the system When a network interface is disconnected from the system (e.g., physically removed if it's a USB adapter), probert asynchronously calls the del_link() method. Upon receiving this notification, Subiquity server wants to send an update to the Subiquity clients. The update contains information about the interface that disappeared - which is obtained through a call to netdev_info. Unfortunately, for Wi-Fi and Ethernet interfaces, netdev_info dereferences the NetworkDev.info variable. Interfaces that no longer exist on the system (and also interfaces that do not yet exist), have their "info" variable set to None - so an exception is raised when dereferencing it. Wi-Fi interface: File "subiquitycore/models/network.py", line 227, in netdev_info scan_state=self.info.wlan['scan_state'], AttributeError: 'NoneType' object has no attribute 'wlan' Ethernet interface: File "subiquitycore/models/network.py", line 201, in netdev_info is_connected = bool(self.info.is_connected) AttributeError: 'NoneType' object has no attribute 'is_connected' Fixed by making sure netdev_info does not raise if the dev.info variable is None. This is a valid use-case. Signed-off-by: Olivier Gayot --- subiquitycore/models/network.py | 18 +++++++++++++--- subiquitycore/models/tests/test_network.py | 25 ++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/subiquitycore/models/network.py b/subiquitycore/models/network.py index 587ba056..c73b1b0a 100644 --- a/subiquitycore/models/network.py +++ b/subiquitycore/models/network.py @@ -205,7 +205,12 @@ class NetworkDev(object): def netdev_info(self) -> NetDevInfo: if self.type == "eth": - is_connected = bool(self.info.is_connected) + if self.info is not None: + is_connected = bool(self.info.is_connected) + else: + # If the device has just disappeared, let's pretend it's not + # connected. + is_connected = False else: is_connected = True bond_master = None @@ -230,10 +235,17 @@ class NetworkDev(object): wlan: Optional[WLANStatus] = None if self.type == "wlan": ssid, psk = self.configured_ssid + # If the device has just disappeared, let's pretend it's not + # scanning and has no visible SSID. + scan_state = None + visible_ssids: List[str] = [] + if self.info is not None: + scan_state = self.info.wlan["scan_state"] + visible_ssids = self.info.wlan["visible_ssids"] wlan = WLANStatus( config=WLANConfig(ssid=ssid, psk=psk), - scan_state=self.info.wlan["scan_state"], - visible_ssids=self.info.wlan["visible_ssids"], + scan_state=scan_state, + visible_ssids=visible_ssids, ) dhcp_addresses = self.dhcp_addresses() diff --git a/subiquitycore/models/tests/test_network.py b/subiquitycore/models/tests/test_network.py index ce99f64b..e03333f6 100644 --- a/subiquitycore/models/tests/test_network.py +++ b/subiquitycore/models/tests/test_network.py @@ -13,6 +13,8 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . +from unittest.mock import Mock + from subiquitycore.models.network import NetworkDev from subiquitycore.tests import SubiTestCase @@ -39,3 +41,26 @@ class TestRouteManagement(SubiTestCase): self.nd.remove_routes(6) expected = self.ipv4s self.assertEqual(expected, self.nd.config["routes"]) + + +class TestNetworkDev(SubiTestCase): + def test_netdev_info_eth_inexistent(self): + # LP: #2012659 - just after physically removing an Ethernet interface + # from the system, Subiquity tries to collect information via + # netdev_info. The code would try to dereference dev.info - but it was + # reset to None when the interface got removed. + # In other private reports, the same issue would occur with Wi-Fi + # interfaces. + model = Mock(get_all_netdevs=Mock(return_value=[])) + nd = NetworkDev(model, "testdev0", "eth") + info = nd.netdev_info() + self.assertFalse(info.is_connected) + + def test_netdev_info_wlan_inexistent(self): + # Just like test_netdev_info_eth_inexistent but with Wi-Fi interfaces + # which suffer the same issue. + model = Mock(get_all_netdevs=Mock(return_value=[])) + nd = NetworkDev(model, "testdev0", "wlan") + info = nd.netdev_info() + self.assertIsNone(info.wlan.scan_state) + self.assertEqual(info.wlan.visible_ssids, [])