From 9d60cd2813fc8f9f89e7c0904cc17336a39e4592 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 27 Jan 2022 06:05:42 +0100 Subject: [PATCH] dhcp: fix crash accepting leases without addresses For IPv6 the lease doesn't necessarily have an address. If the address is missing or the DHCP client doesn't implement accept(), we don't need to wait for the address in platform. From https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1066#note_1233210 : 0 0x00007ffff760f88c in __pthread_kill_implementation () at /lib64/libc.so.6 1 0x00007ffff75c26a6 in raise () at /lib64/libc.so.6 2 0x00007ffff75ac7d3 in abort () at /lib64/libc.so.6 3 0x00007ffff77c5d4c in g_assertion_message (domain=, file=, line=, func=, message=) at ../glib/gtestutils.c:3223 4 0x00007ffff782645f in g_assertion_message_expr (domain=domain@entry=0x5555559e7c96 "nm", file=file@entry=0x5555559deac0 "src/core/dhcp/nm-dhcp-client.c", line=line@entry=609, func=func@entry=0x5555559e0090 <__func__.31> "l3_cfg_notify_cb", expr=expr@entry=0x5555559df5cf "lease_address") at ../glib/gtestutils.c:3249 5 0x00005555558b2866 in l3_cfg_notify_cb (l3cfg=0x555555c29790, notify_data=, self=0x555555e9a1b0) at src/core/dhcp/nm-dhcp-client.c:609 9 0x00007ffff791abe3 in (instance=instance@entry=0x555555c29790, signal_id=, detail=detail@entry=0) at ../gobject/gsignal.c:3553 6 0x00007ffff78fcc7f in g_closure_invoke (closure=0x555555ca3900, return_value=0x0, n_param_values=2, param_values=0x7fffffffd420, invocation_hint=0x7fffffffd3a0) at ../gobject/gclosure.c:830 7 0x00007ffff7919106 in signal_emit_unlocked_R (node=node@entry=0x555555bbadc0, detail=detail@entry=0, instance=instance@entry=0x555555c29790, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fffffffd420) at ../gobject/gsignal.c:3742 8 0x00007ffff791a9ca in g_signal_emit_valist (instance=, signal_id=, detail=, var_args=var_args@entry=0x7fffffffd5f0) at ../gobject/gsignal.c:3497 10 0x000055555564c8a6 in _nm_l3cfg_emit_signal_notify (self=self@entry=0x555555c29790, notify_data=notify_data@entry=0x7fffffffdf30) at src/core/nm-l3cfg.c:576 11 0x000055555564ce77 in _nm_l3cfg_emit_signal_notify_simple (self=self@entry=0x555555c29790, notify_type=notify_type@entry=NM_L3_CONFIG_NOTIFY_TYPE_POST_COMMIT) at src/core/nm-l3cfg.c:585 12 0x0000555555656082 in _l3_commit (self=self@entry=0x555555c29790, commit_type=NM_L3_CFG_COMMIT_TYPE_UPDATE, commit_type@entry=NM_L3_CFG_COMMIT_TYPE_AUTO, is_idle=is_idle@entry=1) at src/core/nm-l3cfg.c:4201 13 0x0000555555656189 in _l3_commit_on_idle_cb (user_data=user_data@entry=0x555555c29790) at src/core/nm-l3cfg.c:2961 14 0x00007ffff77f847b in g_idle_dispatch (source=0x555555d65680, callback=0x55555565612c <_l3_commit_on_idle_cb>, user_data=0x555555c29790) at ../glib/gmain.c:5897 15 0x00007ffff77fc130 in g_main_dispatch (context=0x555555aa5020) at ../glib/gmain.c:3381 16 g_main_context_dispatch (context=0x555555aa5020) at ../glib/gmain.c:4099 17 0x00007ffff7851208 in g_main_context_iterate.constprop.0 (context=0x555555aa5020, block=block@entry=1, dispatch=dispatch@entry=1, self=) at ../glib/gmain.c:4175 18 0x00007ffff77fb853 in g_main_loop_run (loop=0x555555aa5970) at ../glib/gmain.c:4373 19 0x0000555555593c56 in main (argc=, argv=) at src/core/main.c:509 Fixes: e1648d0665a0 ('core: commit l3cd asynchronously on DHCP bound event') https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1074 --- src/core/devices/nm-device.c | 31 ++++++++++++++++++------------- src/core/dhcp/nm-dhcp-client.c | 14 ++++++++++---- 2 files changed, 28 insertions(+), 17 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 3bb78c4b4a..bc23ef0198 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -9956,8 +9956,25 @@ _dev_ipdhcpx_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_d return; } - if (notify_data->lease_update.accepted) { + if (notify_data->lease_update.accepted) _LOGT_ipdhcp(addr_family, "lease accepted"); + else + _LOGT_ipdhcp(addr_family, "lease update"); + + nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, + notify_data->lease_update.l3cd); + + /* Schedule a commit of the configuration. If the DHCP client + * needs to accept the lease, it will send later a LEASE_UPDATE + * notification with accepted=1 once the address appears in platform. + * Otherwise, this notification already has accepted=1. */ + _dev_l3_register_l3cds_set_one_full(self, + L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4), + notify_data->lease_update.l3cd, + NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE, + FALSE); + + if (notify_data->lease_update.accepted) { if (priv->ipdhcp_data_x[IS_IPv4].state != NM_DEVICE_IP_STATE_READY) { _dev_ipdhcpx_set_state(self, addr_family, NM_DEVICE_IP_STATE_READY); nm_dispatcher_call_device(NM_DISPATCHER_ACTION_DHCP_CHANGE_X(IS_IPv4), @@ -9968,20 +9985,8 @@ _dev_ipdhcpx_notify(NMDhcpClient *client, const NMDhcpClientNotifyData *notify_d NULL); _dev_ip_state_check_async(self, addr_family); } - return; } - /* Schedule a commit of the configuration. The DHCP client - * will accept the lease once the address is committed, and - * will send a LEASE_UPDATE notification with accepted=1. */ - _LOGT_ipdhcp(addr_family, "lease update"); - nm_dhcp_config_set_lease(priv->ipdhcp_data_x[IS_IPv4].config, - notify_data->lease_update.l3cd); - _dev_l3_register_l3cds_set_one_full(self, - L3_CONFIG_DATA_TYPE_DHCP_X(IS_IPv4), - notify_data->lease_update.l3cd, - NM_L3CFG_CONFIG_FLAGS_FORCE_ONCE, - FALSE); return; } diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 54f9ee59fd..ec910389f4 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -379,18 +379,24 @@ nm_dhcp_client_set_state(NMDhcpClient *self, NMDhcpState new_state, const NML3Co * as a static address (bypassing ACD), then NML3Cfg is aware of that and signals * immediate success. */ + if (nm_dhcp_client_can_accept(self) && new_state == NM_DHCP_STATE_BOUND && priv->l3cd + && nm_l3_config_data_get_num_addresses(priv->l3cd, priv->config.addr_family) > 0) { + priv->l3cfg_notify.wait_dhcp_commit = TRUE; + } else { + priv->l3cfg_notify.wait_dhcp_commit = FALSE; + } + connect_l3cfg_notify(self); + { const NMDhcpClientNotifyData notify_data = { .notify_type = NM_DHCP_CLIENT_NOTIFY_TYPE_LEASE_UPDATE, .lease_update = { - .l3cd = priv->l3cd, + .l3cd = priv->l3cd, + .accepted = !priv->l3cfg_notify.wait_dhcp_commit, }, }; - priv->l3cfg_notify.wait_dhcp_commit = (new_state == NM_DHCP_STATE_BOUND); - connect_l3cfg_notify(self); - _emit_notify(self, ¬ify_data); } }