From 43016d6ebd232bb8285f3adb386041100679f362 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 7 Feb 2020 10:56:44 +0100 Subject: [PATCH 1/2] n-dhcp4: avoid double free of NDhcp4Outgoing n_dhcp4_c_connection_start_request() should take ownership of the request only on success. On failure the request is freed by the caller. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/355 --- shared/n-dhcp4/src/n-dhcp4-c-connection.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-c-connection.c b/shared/n-dhcp4/src/n-dhcp4-c-connection.c index 3dd0535678..d4354467d1 100644 --- a/shared/n-dhcp4/src/n-dhcp4-c-connection.c +++ b/shared/n-dhcp4/src/n-dhcp4-c-connection.c @@ -1104,13 +1104,14 @@ int n_dhcp4_c_connection_start_request(NDhcp4CConnection *connection, if (request->userdata.start_time == 0) request->userdata.start_time = timestamp; - n_dhcp4_outgoing_free(connection->request); - connection->request = request; + connection->request = n_dhcp4_outgoing_free(connection->request); r = n_dhcp4_c_connection_send_request(connection, request, timestamp); if (r) return r; + connection->request = request; + return 0; } From 45521b1b3872e1f95cb3863f0eae0f8e2408ea42 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 7 Feb 2020 11:00:48 +0100 Subject: [PATCH 2/2] dhcp: nettools: move to failed state if event dispatch fails --- src/dhcp/nm-dhcp-nettools.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-nettools.c b/src/dhcp/nm-dhcp-nettools.c index 23bdefabb8..d3a9956260 100644 --- a/src/dhcp/nm-dhcp-nettools.c +++ b/src/dhcp/nm-dhcp-nettools.c @@ -1063,8 +1063,18 @@ dhcp4_event_cb (int fd, int r; r = n_dhcp4_client_dispatch (priv->client); - if (r < 0) - return G_SOURCE_CONTINUE; + if (r < 0) { + /* FIXME: if any operation (e.g. send()) fails during the + * dispatch, n-dhcp4 returns an error without arming timers + * or progressing state, so the only reasonable thing to do + * is to move to failed state so that the client will be + * restarted. Ideally n-dhcp4 should retry failed operations + * a predefined number of times (possibly infinite). + */ + _LOGE ("error %d dispatching events", r); + nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); + return G_SOURCE_REMOVE; + } while (!n_dhcp4_client_pop_event (priv->client, &event) && event) { dhcp4_event_handle (self, event);