From ae117c588df4bbfaa886600eeec0172e8dc3c7bf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 6 Aug 2021 09:25:34 +0200 Subject: [PATCH] dhcp: ensure NMDhcpClient was stopped in destructor The user of NMDhcpClient is supposed to call "nm_dhcp_client_stop()" on the instance, also because it is a ref-counted GObject type. So we wouldn't want to rely on the last unref to stop DHCP. Still, in case that was not done, the code somehow made the assumption it made any sense to possibly not stop the DHCP client. For the internal client, there is of course nothing left after destroying the NMDhcpClient instance, but what about dhclient? I don't think we should ever leave dhclient running unsupervised. Even during restart of the service, we need to stop it first, and restart it afterwards. When we quit NetworkManager, we may want to leave the interface up and configured. For that, we may need to take care that "nm_dhcp_client_stop()" does not destory the IP configuration of the intrerface. But I don't think that is a problem. What we still want to do, is kill the dhclient instance. NMDhcpClient is not supposed to be ever restarted. It starts when an instance gets created, and it stops with "nm_dhcp_client_stop()". Hence, the simple "is_stopped" flag is fine to prevent that multiple stop calls are harmful. --- src/core/dhcp/nm-dhcp-client.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/core/dhcp/nm-dhcp-client.c b/src/core/dhcp/nm-dhcp-client.c index 0985e386e1..f88c79c0be 100644 --- a/src/core/dhcp/nm-dhcp-client.c +++ b/src/core/dhcp/nm-dhcp-client.c @@ -78,6 +78,7 @@ typedef struct _NMDhcpClientPrivate { NMDhcpHostnameFlags hostname_flags; NMDhcpClientFlags client_flags; bool iaid_explicit : 1; + bool is_stopped : 1; } NMDhcpClientPrivate; G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT) @@ -774,6 +775,11 @@ nm_dhcp_client_stop(NMDhcpClient *self, gboolean release) priv = NM_DHCP_CLIENT_GET_PRIVATE(self); + if (priv->is_stopped) + return; + + priv->is_stopped = TRUE; + /* Kill the DHCP client */ old_pid = priv->pid; NM_DHCP_CLIENT_GET_CLASS(self)->stop(self, release); @@ -1222,10 +1228,7 @@ dispose(GObject *object) NMDhcpClient * self = NM_DHCP_CLIENT(object); NMDhcpClientPrivate *priv = NM_DHCP_CLIENT_GET_PRIVATE(self); - /* Stopping the client is left up to the controlling device - * explicitly since we may want to quit NetworkManager but not terminate - * the DHCP client. - */ + nm_dhcp_client_stop(self, FALSE); watch_cleanup(self); timeout_cleanup(self);