From 565b389bf6d6ae2a90461a9f6e214fac11d714c4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 27 May 2022 17:11:02 +0200 Subject: [PATCH 1/2] core: add nm_l3cfg_block_obj_pruning() Add a function prevent the removal of addresses and routes from the interface for a given address family. (cherry picked from commit e8275d713921a8f663d16bee0607756c50dcfa5a) (cherry picked from commit 59ef1b4c78b3c42ccad08bba7bec7c662e3d8fcf) --- src/core/nm-l3cfg.c | 95 ++++++++++++++++++++++++++++++++++++++------- src/core/nm-l3cfg.h | 5 +++ 2 files changed, 87 insertions(+), 13 deletions(-) diff --git a/src/core/nm-l3cfg.c b/src/core/nm-l3cfg.c index 5f87dff808..6dee997f83 100644 --- a/src/core/nm-l3cfg.c +++ b/src/core/nm-l3cfg.c @@ -208,6 +208,12 @@ typedef struct { gboolean force_commit_once : 1; } L3ConfigData; +struct _NML3CfgBlockHandle { + NML3Cfg *self; + CList lst; + gboolean is_ipv4; +}; + /*****************************************************************************/ NM_GOBJECT_PROPERTIES_DEFINE(NML3Cfg, PROP_NETNS, PROP_IFINDEX, ); @@ -249,6 +255,14 @@ typedef struct _NML3CfgPrivate { GSource *nacd_event_down_source; gint64 nacd_event_down_ratelimited_until_msec; + union { + struct { + CList blocked_lst_head_6; + CList blocked_lst_head_4; + }; + CList blocked_lst_head_x[2]; + }; + union { struct { NMIPConfig *ipconfig_6; @@ -4242,20 +4256,29 @@ _l3_commit_one(NML3Cfg *self, g_array_append_val(ipv6_temp_addrs_keep, addr->address); } } - addresses_prune = - nm_platform_ip_address_get_prune_list(self->priv.platform, - addr_family, - self->priv.ifindex, - nm_g_array_data(ipv6_temp_addrs_keep), - nm_g_array_len(ipv6_temp_addrs_keep)); - routes_prune = nm_platform_ip_route_get_prune_list(self->priv.platform, - addr_family, - self->priv.ifindex, - route_table_sync); - _obj_state_zombie_lst_prune_all(self, addr_family); - } else - _obj_state_zombie_lst_get_prune_lists(self, addr_family, &addresses_prune, &routes_prune); + if (c_list_is_empty(&self->priv.p->blocked_lst_head_x[IS_IPv4])) { + addresses_prune = + nm_platform_ip_address_get_prune_list(self->priv.platform, + addr_family, + self->priv.ifindex, + nm_g_array_data(ipv6_temp_addrs_keep), + nm_g_array_len(ipv6_temp_addrs_keep)); + + routes_prune = nm_platform_ip_route_get_prune_list(self->priv.platform, + addr_family, + self->priv.ifindex, + route_table_sync); + _obj_state_zombie_lst_prune_all(self, addr_family); + } + } else { + if (c_list_is_empty(&self->priv.p->blocked_lst_head_x[IS_IPv4])) { + _obj_state_zombie_lst_get_prune_lists(self, + addr_family, + &addresses_prune, + &routes_prune); + } + } /* FIXME(l3cfg): need to honor and set nm_l3_config_data_get_ndisc_*(). */ /* FIXME(l3cfg): need to honor and set nm_l3_config_data_get_mtu(). */ @@ -4376,6 +4399,48 @@ _l3_commit(NML3Cfg *self, NML3CfgCommitType commit_type, gboolean is_idle) _nm_l3cfg_emit_signal_notify_simple(self, NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT); } +NML3CfgBlockHandle * +nm_l3cfg_block_obj_pruning(NML3Cfg *self, int addr_family) +{ + const int IS_IPv4 = NM_IS_IPv4(addr_family); + NML3CfgBlockHandle *handle; + + if (!self) + return NULL; + + nm_assert(NM_IS_L3CFG(self)); + + handle = g_slice_new(NML3CfgBlockHandle); + handle->self = g_object_ref(self); + handle->is_ipv4 = IS_IPv4; + c_list_link_tail(&self->priv.p->blocked_lst_head_x[IS_IPv4], &handle->lst); + + _LOGT("obj-pruning for IPv%c: blocked (%zu)", + nm_utils_addr_family_to_char(addr_family), + c_list_length(&self->priv.p->blocked_lst_head_x[IS_IPv4])); + + return handle; +} + +void +nm_l3cfg_unblock_obj_pruning(NML3CfgBlockHandle *handle) +{ + gs_unref_object NML3Cfg *self = handle->self; + const int IS_IPv4 = handle->is_ipv4; + + nm_assert(NM_IS_L3CFG(self)); + nm_assert(c_list_is_linked(&handle->lst)); + nm_assert(c_list_contains(&self->priv.p->blocked_lst_head_x[IS_IPv4], &handle->lst)); + + c_list_unlink_stale(&handle->lst); + + _LOGT("obj-pruning for IPv%c: unblocked (%zu)", + IS_IPv4 ? '4' : '6', + c_list_length(&self->priv.p->blocked_lst_head_x[IS_IPv4])); + + nm_g_slice_free(handle); +} + /* See DOC(l3cfg:commit-type) */ void nm_l3cfg_commit(NML3Cfg *self, NML3CfgCommitType commit_type) @@ -4680,6 +4745,8 @@ nm_l3cfg_init(NML3Cfg *self) c_list_init(&self->priv.p->obj_state_lst_head); c_list_init(&self->priv.p->obj_state_temporary_not_available_lst_head); c_list_init(&self->priv.p->obj_state_zombie_lst_head); + c_list_init(&self->priv.p->blocked_lst_head_4); + c_list_init(&self->priv.p->blocked_lst_head_6); self->priv.p->obj_state_hash = g_hash_table_new_full(nmp_object_indirect_id_hash, nmp_object_indirect_id_equal, @@ -4728,6 +4795,8 @@ finalize(GObject *object) nm_assert(!self->priv.p->ipv4ll); nm_assert(c_list_is_empty(&self->priv.p->commit_type_lst_head)); + nm_assert(c_list_is_empty(&self->priv.p->blocked_lst_head_4)); + nm_assert(c_list_is_empty(&self->priv.p->blocked_lst_head_6)); nm_assert(!self->priv.p->commit_on_idle_source); diff --git a/src/core/nm-l3cfg.h b/src/core/nm-l3cfg.h index f6ec39ced8..215d21e993 100644 --- a/src/core/nm-l3cfg.h +++ b/src/core/nm-l3cfg.h @@ -455,4 +455,9 @@ struct _NMIPConfig *nm_l3cfg_ipconfig_acquire(NML3Cfg *self, int addr_family); /*****************************************************************************/ +typedef struct _NML3CfgBlockHandle NML3CfgBlockHandle; + +NML3CfgBlockHandle *nm_l3cfg_block_obj_pruning(NML3Cfg *self, int addr_family); +void nm_l3cfg_unblock_obj_pruning(NML3CfgBlockHandle *handle); + #endif /* __NM_L3CFG_H__ */ From 813b5dfb4b92bb6765cf4a518a9b25845155b100 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 13 Jun 2022 11:21:15 +0200 Subject: [PATCH 2/2] ppp: don't remove addresses from interface while IPCP/IPV6CP is running pppd also tries to configure addresses by itself through some ioctls. If we remove between those calls an address that was added, pppd fails and quits. To avoid this race condition, don't remove addresses while IPCP and IPV6CP are running. Once pppd sends an IP configuration, it has finished configuring the interface and we can proceed normally. https://bugzilla.redhat.com/show_bug.cgi?id=2085382 (cherry picked from commit b41b11d613a2ed466975eaf4b7a13ff67662a772) (cherry picked from commit e95b44bacbafe577111dcefc50f1775e3345214e) --- src/core/devices/nm-device-ppp.c | 37 +++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/core/devices/nm-device-ppp.c b/src/core/devices/nm-device-ppp.c index 61e32348c4..6615b65e02 100644 --- a/src/core/devices/nm-device-ppp.c +++ b/src/core/devices/nm-device-ppp.c @@ -23,6 +23,13 @@ typedef struct _NMDevicePppPrivate { NMPppMgr *ppp_mgr; + union { + struct { + NML3CfgBlockHandle *l3cfg_block_handle_6; + NML3CfgBlockHandle *l3cfg_block_handle_4; + }; + NML3CfgBlockHandle *l3cfg_block_handle_x[2]; + }; } NMDevicePppPrivate; struct _NMDevicePpp { @@ -69,8 +76,10 @@ _ppp_mgr_stage3_maybe_ready(NMDevicePpp *self) const NMPppMgrIPData *ip_data; ip_data = nm_ppp_mgr_get_ip_data(priv->ppp_mgr, addr_family); - if (ip_data->ip_received) + if (ip_data->ip_received) { + nm_clear_pointer(&priv->l3cfg_block_handle_x[IS_IPv4], nm_l3cfg_unblock_obj_pruning); nm_device_devip_set_state(device, addr_family, NM_DEVICE_IP_STATE_READY, ip_data->l3cd); + } } if (nm_ppp_mgr_get_state(priv->ppp_mgr) >= NM_PPP_MGR_STATE_HAVE_IP_CONFIG) @@ -80,9 +89,10 @@ _ppp_mgr_stage3_maybe_ready(NMDevicePpp *self) static void _ppp_mgr_callback(NMPppMgr *ppp_mgr, const NMPppMgrCallbackData *callback_data, gpointer user_data) { - NMDevicePpp *self = NM_DEVICE_PPP(user_data); - NMDevice *device = NM_DEVICE(self); - NMDeviceState device_state; + NMDevicePpp *self = NM_DEVICE_PPP(user_data); + NMDevice *device = NM_DEVICE(self); + NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE(self); + NMDeviceState device_state; if (callback_data->callback_type != NM_PPP_MGR_CALLBACK_TYPE_STATE_CHANGED) return; @@ -112,6 +122,19 @@ _ppp_mgr_callback(NMPppMgr *ppp_mgr, const NMPppMgrCallbackData *callback_data, return; } + /* pppd also tries to configure addresses by itself through some + * ioctls. If we remove between those calls an address that was added, + * pppd fails and quits. Temporarily block the removal of addresses + * and routes. */ + if (!priv->l3cfg_block_handle_4) { + priv->l3cfg_block_handle_4 = + nm_l3cfg_block_obj_pruning(nm_device_get_l3cfg(device), AF_INET); + } + if (!priv->l3cfg_block_handle_6) { + priv->l3cfg_block_handle_6 = + nm_l3cfg_block_obj_pruning(nm_device_get_l3cfg(device), AF_INET6); + } + if (old_name) nm_manager_remove_device(NM_MANAGER_GET, old_name, NM_DEVICE_TYPE_PPP); @@ -257,7 +280,11 @@ create_and_realize(NMDevice *device, static void deactivate(NMDevice *device) { - NMDevicePpp *self = NM_DEVICE_PPP(device); + NMDevicePpp *self = NM_DEVICE_PPP(device); + NMDevicePppPrivate *priv = NM_DEVICE_PPP_GET_PRIVATE(self); + + nm_clear_pointer(&priv->l3cfg_block_handle_4, nm_l3cfg_unblock_obj_pruning); + nm_clear_pointer(&priv->l3cfg_block_handle_6, nm_l3cfg_unblock_obj_pruning); _ppp_mgr_cleanup(self); }