device: fix crash releasing destroyed slave

I encountered this on a WIP branch, but I think it can happen
under regular conditions. I think there is no error condition here,
and we should do nothing if we have no ifindex.

  <debug> [1561653068.2192] platform: signal: link removed: 1699: test1p <DOWN;broadcast,multicast> mtu 1500 master 1698 arp 1 veth* init addrgenmode none addr D6:14:45:97:06:75 brd FF:FF:FF:FF:FF:FF driver veth rx:0,0 tx:38,5606
  ...
  <info>  [1561653068.2617] device (test1): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed')
  ...
  <trace> [1561653068.2635] device[0x564058c73750] (test1p): sys-iface-state: external -> removed
  <debug> [1561653068.2635] device[0x564058c73750] (test1p): unrealize (ifindex 1699)
  <debug> [1561653068.2636] device[0x564058c73750] (test1p): parent: clear
  <trace> [1561653068.2636] device[0x564058b98eb0] (vethbr): mtu: commit-mtu...
  <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!by-type,!user-explicit,!user-settings,!user-udev,!is-slave=0x10/0x1479/unmanaged/unrealized], set-unmanaged [platform-init=0x10])
  <debug> [1561653068.2639] device[0x564058c73750] (test1p): unmanaged: flags set to [platform-init,!sleeping,!user-settings=0x10/0x51/unmanaged/unrealized], forget [parent,by-type,user-explicit,user-udev,external-down,is-slave=0x1c2c])
  <info>  [1561653068.2639] device (test1p): state change: activated -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed')
  <debug> [1561653068.2640] device[0x564058c73750] (test1p): deactivating device (reason 'unmanaged') [3]
  <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip4-state: set to 0 (none)
  <trace> [1561653068.2640] device[0x564058c73750] (test1p): ip6-state: set to 0 (none)
  <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'dhcp6' not pending (expected)
  <trace> [1561653068.2640] device[0x564058c73750] (test1p): remove_pending_action (0): 'autoconf6' not pending (expected)
  <debug> [1561653068.2640] rules-manager: sync
  <debug> [1561653068.2640] device[0x564058c73750] (test1p): set metered value 0
  <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip4-config: update (commit=1, new-config=(nil))
  <debug> [1561653068.2641] device[0x564058c73750] (test1p): ip6-config: update (commit=1, new-config=(nil))
  <debug> [1561653068.2644] device[0x564058b98eb0] (vethbr): slave test1p state change 100 (activated) -> 10 (unmanaged)
  <trace> [1561653068.2644] device[0x564058b98eb0] (vethbr): master: release one slave 0x564058c73750/test1p
  ((src/platform/nm-platform.c:2002)): assertion '<dropped>' failed

backtrace:
  ...
  #3  0x0000564057fb713e _nm_g_return_if_fail_warning (NetworkManager)
  #4  0x000056405808b37c release_slave (NetworkManager)
  #5  0x0000564058079aef nm_device_master_release_one_slave (NetworkManager)
  #6  0x00005640580844d7 slave_state_changed (NetworkManager)
  #7  0x00007efc24833fae ffi_call_unix64 (libffi.so.6)
  #8  0x00007efc2483396f ffi_call (libffi.so.6)
  #9  0x00007efc29b836e5 g_cclosure_marshal_generic (libgobject-2.0.so.0)
  #10 0x00007efc29b82c1d g_closure_invoke (libgobject-2.0.so.0)
  #11 0x00007efc29b96173 signal_emit_unlocked_R (libgobject-2.0.so.0)
  #12 0x00007efc29b9f29a g_signal_emit_valist (libgobject-2.0.so.0)
  #13 0x00007efc29b9f893 g_signal_emit (libgobject-2.0.so.0)
  #14 0x000056405807ab20 _set_state_full (NetworkManager)
  #15 0x000056405807d803 nm_device_unrealize (NetworkManager)
  #16 0x0000564057f6072c _platform_link_cb_idle (NetworkManager)
  #17 0x00007efc296a01db g_idle_dispatch (libglib-2.0.so.0)
  ...
This commit is contained in:
Thomas Haller 2019-06-28 17:31:40 +02:00
parent 7c84227c93
commit 5b98f2fb01
3 changed files with 45 additions and 16 deletions

View file

@ -413,6 +413,12 @@ release_slave (NMDevice *device,
NMDeviceBond *self = NM_DEVICE_BOND (device);
gboolean success;
gs_free char *address = NULL;
int ifindex_slave;
ifindex_slave = nm_device_get_ip_ifindex (slave);
if (ifindex_slave <= 0)
_LOGD (LOGD_TEAM, "bond slave %s is already released", nm_device_get_ip_iface (slave));
if (configure) {
/* When the last slave is released the bond MAC will be set to a random
@ -420,16 +426,18 @@ release_slave (NMDevice *device,
*/
address = g_strdup (nm_device_get_hw_address (device));
success = nm_platform_link_release (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
nm_device_get_ip_ifindex (slave));
if (ifindex_slave > 0) {
success = nm_platform_link_release (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
ifindex_slave);
if (success) {
_LOGI (LOGD_BOND, "released bond slave %s",
nm_device_get_ip_iface (slave));
} else {
_LOGW (LOGD_BOND, "failed to release bond slave %s",
nm_device_get_ip_iface (slave));
if (success) {
_LOGI (LOGD_BOND, "released bond slave %s",
nm_device_get_ip_iface (slave));
} else {
_LOGW (LOGD_BOND, "failed to release bond slave %s",
nm_device_get_ip_iface (slave));
}
}
nm_platform_process_events (nm_device_get_platform (device));
@ -440,11 +448,15 @@ release_slave (NMDevice *device,
* IFF_UP), so we must bring it back up here to ensure carrier changes and
* other state is noticed by the now-released slave.
*/
if (!nm_device_bring_up (slave, TRUE, NULL))
_LOGW (LOGD_BOND, "released bond slave could not be brought up.");
if (ifindex_slave > 0) {
if (!nm_device_bring_up (slave, TRUE, NULL))
_LOGW (LOGD_BOND, "released bond slave could not be brought up.");
}
} else {
_LOGI (LOGD_BOND, "bond slave %s was released",
nm_device_get_ip_iface (slave));
if (ifindex_slave > 0) {
_LOGI (LOGD_BOND, "bond slave %s was released",
nm_device_get_ip_iface (slave));
}
}
}

View file

@ -625,11 +625,19 @@ release_slave (NMDevice *device,
{
NMDeviceBridge *self = NM_DEVICE_BRIDGE (device);
gboolean success;
int ifindex_slave;
ifindex_slave = nm_device_get_ip_ifindex (slave);
if (ifindex_slave <= 0) {
_LOGD (LOGD_TEAM, "bond slave %s is already released", nm_device_get_ip_iface (slave));
return;
}
if (configure) {
success = nm_platform_link_release (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
nm_device_get_ip_ifindex (slave));
ifindex_slave);
if (success) {
_LOGI (LOGD_BRIDGE, "detached bridge port %s",

View file

@ -774,11 +774,19 @@ release_slave (NMDevice *device,
NMDeviceTeam *self = NM_DEVICE_TEAM (device);
NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self);
gboolean success;
int ifindex_slave;
ifindex_slave = nm_device_get_ip_ifindex (slave);
if (ifindex_slave <= 0) {
_LOGD (LOGD_TEAM, "team port %s is already released", nm_device_get_ip_iface (slave));
return;
}
if (configure) {
success = nm_platform_link_release (nm_device_get_platform (device),
nm_device_get_ip_ifindex (device),
nm_device_get_ip_ifindex (slave));
ifindex_slave);
if (success)
_LOGI (LOGD_TEAM, "released team port %s", nm_device_get_ip_iface (slave));
@ -789,9 +797,10 @@ release_slave (NMDevice *device,
* IFF_UP), so we must bring it back up here to ensure carrier changes and
* other state is noticed by the now-released port.
*/
if (!nm_device_bring_up (slave, TRUE, NULL))
if (!nm_device_bring_up (slave, TRUE, NULL)) {
_LOGW (LOGD_TEAM, "released team port %s could not be brought up",
nm_device_get_ip_iface (slave));
}
nm_clear_g_source (&priv->teamd_read_timeout);
priv->teamd_read_timeout = g_timeout_add_seconds (5,