Skip to content

Commit

Permalink
network: fix crash when Wi-Fi or eth interface gets removed from the …
Browse files Browse the repository at this point in the history
…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 <olivier.gayot@canonical.com>
  • Loading branch information
ogayot committed Sep 7, 2023
1 parent 4d6fb69 commit e10343b
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
18 changes: 15 additions & 3 deletions subiquitycore/models/network.py
Expand Up @@ -205,7 +205,12 @@ def __init__(self, model, name, typ):

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
Expand All @@ -230,10 +235,17 @@ def netdev_info(self) -> NetDevInfo:
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()
Expand Down
25 changes: 25 additions & 0 deletions subiquitycore/models/tests/test_network.py
Expand Up @@ -13,6 +13,8 @@
# You should have received a copy of the GNU Affero General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from unittest.mock import Mock

from subiquitycore.models.network import NetworkDev
from subiquitycore.tests import SubiTestCase

Expand All @@ -39,3 +41,26 @@ def test_remove_v6(self):
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, [])

0 comments on commit e10343b

Please sign in to comment.