[PATCH v2] gsupplicant: Mem leak in wpa_s because "RemoveNetwork" not called
by Naveen Singh
From: nasingh <naveensingh0977(a)gmail.com>
Connman did not call netwok_remove in case AP deauthenticated client causing
wpa_s to re-allocate the ssid pointer even if the next connection attempt
is for the same SSID. This change ensures that at the time of connection
(DBUS Method call AddNetwork) if the network is found not removed, it calls
the dbus API to remove the network and once network is removed, proceed with
the connection.
Tested by running a deauth loop script at the AP end and ensure that wpa_s does
not allocate memory for struct wpa_ssid for all the subsequent
connection attempts. During the test memory usage of wpa_s is monitored.
---
gsupplicant/supplicant.c | 158 +++++++++++++++++++++++++++++++++--------------
1 file changed, 110 insertions(+), 48 deletions(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 342cb01..d13467e 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -249,6 +249,50 @@ struct _GSupplicantGroup {
GSList *members;
};
+struct interface_data {
+ GSupplicantInterface *interface;
+ char *path; /* Interface path cannot be taken from interface (above) as
+ * it might have been freed already.
+ */
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ bool network_remove_in_progress;
+ GSupplicantSSID *ssid;
+};
+
+struct interface_create_data {
+ char *ifname;
+ char *driver;
+ char *bridge;
+ GSupplicantInterface *interface;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+};
+
+struct interface_connect_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ union {
+ GSupplicantSSID *ssid;
+ GSupplicantPeerParams *peer;
+ };
+};
+
+struct interface_scan_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ GSupplicantScanParams *scan_params;
+ void *user_data;
+};
+
+static int network_remove(struct interface_data *data);
+static void network_remove_params(DBusMessageIter *iter, void *user_data);
+static void network_remove_result(const char *error,
+ DBusMessageIter *iter, void *user_data);
+
static inline void debug(const char *format, ...)
{
char str[256];
@@ -3476,43 +3520,6 @@ GSupplicantPeer *g_supplicant_interface_peer_lookup(GSupplicantInterface *interf
return peer;
}
-struct interface_data {
- GSupplicantInterface *interface;
- char *path; /* Interface path cannot be taken from interface (above) as
- * it might have been freed already.
- */
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_create_data {
- char *ifname;
- char *driver;
- char *bridge;
- GSupplicantInterface *interface;
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_connect_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- union {
- GSupplicantSSID *ssid;
- GSupplicantPeerParams *peer;
- };
- void *user_data;
-};
-
-struct interface_scan_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- GSupplicantScanParams *scan_params;
- void *user_data;
-};
-
static void interface_create_data_free(struct interface_create_data *data)
{
g_free(data->ifname);
@@ -4105,7 +4112,6 @@ static void interface_add_network_result(const char *error,
SUPPLICANT_DBG("PATH: %s", path);
- g_free(interface->network_path);
interface->network_path = g_strdup(path);
supplicant_dbus_method_call(data->interface->path,
@@ -4656,7 +4662,8 @@ int g_supplicant_interface_connect(GSupplicantInterface *interface,
void *user_data)
{
struct interface_connect_data *data;
- int ret;
+ struct interface_data *intf_data;
+ int ret = 0;
if (!interface)
return -EINVAL;
@@ -4685,12 +4692,45 @@ int g_supplicant_interface_connect(GSupplicantInterface *interface,
SUPPLICANT_INTERFACE ".Interface.WPS",
"ProcessCredentials", DBUS_TYPE_BOOLEAN_AS_STRING,
wps_process_credentials, wps_start, data, interface);
- } else
- ret = supplicant_dbus_method_call(interface->path,
- SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
- interface_add_network_params,
- interface_add_network_result, data,
- interface);
+ } else {
+ /* By the time there is a request for connect and the network
+ * path is not NULL it means that connman has not removed the
+ * previous network pointer. This can happen in the case AP
+ * deauthenticated client and connman does not remove the
+ * previously connected network pointer. This causes supplicant
+ * to reallocate the memory for struct wpa_ssid again even if it
+ * is the same SSID. This causes memory usage of wpa_supplicnat
+ * to go high. The idea here is that if the previously connected
+ * network is not removed at the time of next connection attempt
+ * check if the network path is not NULL. In case it is non-NULL
+ * first remove the network and then once removal is successful, add
+ * the network.
+ */
+
+ if (interface->network_path != NULL) {
+ g_free(data->path);
+ dbus_free(data);
+
+ intf_data = dbus_malloc0(sizeof(*intf_data));
+ if (!intf_data)
+ return -ENOMEM;
+
+ intf_data->interface = interface;
+ intf_data->path = g_strdup(interface->path);
+ intf_data->callback = callback;
+ intf_data->ssid = ssid;
+ intf_data->user_data = user_data;
+ intf_data->network_remove_in_progress = TRUE;
+ network_remove(intf_data);
+ }
+ else {
+ ret = supplicant_dbus_method_call(interface->path,
+ SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, data,
+ interface);
+ }
+ }
if (ret < 0) {
g_free(data->path);
@@ -4705,6 +4745,7 @@ static void network_remove_result(const char *error,
DBusMessageIter *iter, void *user_data)
{
struct interface_data *data = user_data;
+ struct interface_connect_data * connect_data;
int result = 0;
SUPPLICANT_DBG("");
@@ -4716,11 +4757,32 @@ static void network_remove_result(const char *error,
result = -ECONNABORTED;
}
- g_free(data->path);
+ g_free(data->interface->network_path);
+ data->interface->network_path = NULL;
- if (data->callback)
- data->callback(result, data->interface, data->user_data);
+ if (data->network_remove_in_progress == TRUE) {
+ data->network_remove_in_progress = FALSE;
+ connect_data = dbus_malloc0(sizeof(*connect_data));
+ if (!connect_data)
+ return;
+
+ connect_data->interface = data->interface;
+ connect_data->path = data->path;
+ connect_data->callback = data->callback;
+ connect_data->ssid = data->ssid;
+ connect_data->user_data = data->user_data;
+ supplicant_dbus_method_call(data->interface->path,
+ SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, connect_data,
+ connect_data->interface);
+ }
+ else {
+ g_free(data->path);
+ if (data->callback)
+ data->callback(result, data->interface, data->user_data);
+ }
dbus_free(data);
}
--
2.7.0.rc3.207.g0ac5344
4 years, 8 months
[PATCH] gsupplicant: Mem leak in wpa_s because "RemoveNetwork" not called
by Naveen Singh
From: nasingh <naveensingh0977(a)gmail.com>
Connman did not call netwok_remove in case AP deauthenticated client causing
wpa_s to re-allocate the ssid pointer even if the next connection attempt
is for the same SSID. This change ensures that at the time of connection
(DBUS Method call AddNetwork) if the network is found not removed, it calls
the dbus API to remove the network and once network is removed, proceed with
the connection.
Tested by running a deauth loop script at the AP end and ensure that wpa_s does
not allocate memory for struct wpa_ssid for all the subsequent
connection attempts. During the test memory usage of wpa_s is monitored.
---
gsupplicant/supplicant.c | 139 ++++++++++++++++++++++++++++++-----------------
1 file changed, 89 insertions(+), 50 deletions(-)
diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c
index 342cb01..b954e91 100644
--- a/gsupplicant/supplicant.c
+++ b/gsupplicant/supplicant.c
@@ -249,6 +249,51 @@ struct _GSupplicantGroup {
GSList *members;
};
+struct interface_data {
+ GSupplicantInterface *interface;
+ char *path; /* Interface path cannot be taken from interface (above) as
+ * it might have been freed already.
+ */
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ bool network_remove_in_progress;
+};
+
+struct interface_create_data {
+ char *ifname;
+ char *driver;
+ char *bridge;
+ GSupplicantInterface *interface;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+};
+
+struct interface_connect_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ void *user_data;
+ bool network_remove_in_progress;
+ union {
+ GSupplicantSSID *ssid;
+ GSupplicantPeerParams *peer;
+ };
+};
+
+struct interface_scan_data {
+ GSupplicantInterface *interface;
+ char *path;
+ GSupplicantInterfaceCallback callback;
+ GSupplicantScanParams *scan_params;
+ void *user_data;
+};
+
+
+static int network_remove(struct interface_data *data);
+static void network_remove_params(DBusMessageIter *iter, void *user_data);
+static void network_remove_result(const char *error,
+ DBusMessageIter *iter, void *user_data);
+
static inline void debug(const char *format, ...)
{
char str[256];
@@ -3476,43 +3521,6 @@ GSupplicantPeer *g_supplicant_interface_peer_lookup(GSupplicantInterface *interf
return peer;
}
-struct interface_data {
- GSupplicantInterface *interface;
- char *path; /* Interface path cannot be taken from interface (above) as
- * it might have been freed already.
- */
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_create_data {
- char *ifname;
- char *driver;
- char *bridge;
- GSupplicantInterface *interface;
- GSupplicantInterfaceCallback callback;
- void *user_data;
-};
-
-struct interface_connect_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- union {
- GSupplicantSSID *ssid;
- GSupplicantPeerParams *peer;
- };
- void *user_data;
-};
-
-struct interface_scan_data {
- GSupplicantInterface *interface;
- char *path;
- GSupplicantInterfaceCallback callback;
- GSupplicantScanParams *scan_params;
- void *user_data;
-};
-
static void interface_create_data_free(struct interface_create_data *data)
{
g_free(data->ifname);
@@ -4105,7 +4113,6 @@ static void interface_add_network_result(const char *error,
SUPPLICANT_DBG("PATH: %s", path);
- g_free(interface->network_path);
interface->network_path = g_strdup(path);
supplicant_dbus_method_call(data->interface->path,
@@ -4656,7 +4663,7 @@ int g_supplicant_interface_connect(GSupplicantInterface *interface,
void *user_data)
{
struct interface_connect_data *data;
- int ret;
+ int ret = 0;
if (!interface)
return -EINVAL;
@@ -4685,12 +4692,33 @@ int g_supplicant_interface_connect(GSupplicantInterface *interface,
SUPPLICANT_INTERFACE ".Interface.WPS",
"ProcessCredentials", DBUS_TYPE_BOOLEAN_AS_STRING,
wps_process_credentials, wps_start, data, interface);
- } else
- ret = supplicant_dbus_method_call(interface->path,
- SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
- interface_add_network_params,
- interface_add_network_result, data,
- interface);
+ } else {
+ /* By the time there is a request for connect and the network
+ * path is not NULL it means that connman has not removed the
+ * previous network pointer. This can happen in the case AP
+ * deauthenticated client and connman does not remove the
+ * previously connected network pointer. This causes supplicant
+ * to reallocate the memory for struct wpa_ssid again even if it
+ * is the same SSID. This causes memory usage of wpa_supplicnat
+ * to go high. The idea here is that if the previously connected
+ * network is not removed at the time of next connection attempt
+ * check if the network path is not NULL. In case it is non-NULL
+ * first remove the network and then once removal is successful, add
+ * the network.
+ */
+
+ if (interface->network_path != NULL) {
+ data->network_remove_in_progress = TRUE;
+ network_remove((struct interface_data *)data);
+ }
+ else {
+ ret = supplicant_dbus_method_call(interface->path,
+ SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, data,
+ interface);
+ }
+ }
if (ret < 0) {
g_free(data->path);
@@ -4716,12 +4744,23 @@ static void network_remove_result(const char *error,
result = -ECONNABORTED;
}
- g_free(data->path);
+ g_free(data->interface->network_path);
+ data->interface->network_path = NULL;
- if (data->callback)
- data->callback(result, data->interface, data->user_data);
-
- dbus_free(data);
+ if (data->network_remove_in_progress == TRUE) {
+ data->network_remove_in_progress = FALSE;
+ supplicant_dbus_method_call(data->interface->path,
+ SUPPLICANT_INTERFACE ".Interface", "AddNetwork",
+ interface_add_network_params,
+ interface_add_network_result, data,
+ data->interface);
+ }
+ else {
+ g_free(data->path);
+ if (data->callback)
+ data->callback(result, data->interface, data->user_data);
+ dbus_free(data);
+ }
}
static void network_remove_params(DBusMessageIter *iter, void *user_data)
--
2.7.0.rc3.207.g0ac5344
4 years, 8 months
[PATCH] gdhcp: use opened listening socket to send DHCP renew request
by Feng Wang
It fix DHCP ACK lost issue when doing DHCP renewal.
When doing DHCP renew, 2 sockets are opened. One is for
listening DHCP ACK, the other is for transmitting DHCP request
which is closed immediately after transmitting is done. But in
some cases, the socket is closed after the DHCP ACK is received.
The kernel will route the packet to the transmitting socket
because it has a better match result(dst ip/port etc). And the
packet was dropped when the socket was closed.
---
gdhcp/client.c | 6 ++++--
gdhcp/common.c | 60 +++++++++++++++++++++++++++++++++++-----------------------
gdhcp/common.h | 2 +-
3 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/gdhcp/client.c b/gdhcp/client.c
index 3bf8cb2..ad587b1 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -502,7 +502,8 @@ static int send_request(GDHCPClient *dhcp_client)
if (dhcp_client->state == RENEWING)
return dhcp_send_kernel_packet(&packet,
dhcp_client->requested_ip, CLIENT_PORT,
- dhcp_client->server_ip, SERVER_PORT);
+ dhcp_client->server_ip, SERVER_PORT,
+ dhcp_client->listener_sockfd);
return dhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT,
INADDR_BROADCAST, SERVER_PORT,
@@ -526,7 +527,8 @@ static int send_release(GDHCPClient *dhcp_client,
dhcp_add_option_uint32(&packet, DHCP_SERVER_ID, server);
return dhcp_send_kernel_packet(&packet, ciaddr, CLIENT_PORT,
- server, SERVER_PORT);
+ server, SERVER_PORT,
+ dhcp_client->listener_sockfd);
}
static gboolean ipv4ll_probe_timeout(gpointer dhcp_data);
diff --git a/gdhcp/common.c b/gdhcp/common.c
index f3d4677..f0a9aa6 100644
--- a/gdhcp/common.c
+++ b/gdhcp/common.c
@@ -626,44 +626,56 @@ int dhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
int dhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
uint32_t source_ip, int source_port,
- uint32_t dest_ip, int dest_port)
+ uint32_t dest_ip, int dest_port, int fd)
{
struct sockaddr_in client;
- int fd, n, opt = 1;
+ int n, opt = 1;
enum {
DHCP_SIZE = sizeof(struct dhcp_packet) -
EXTEND_FOR_BUGGY_SERVERS,
};
- fd = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
- if (fd < 0)
- return -errno;
+ if (fd < 0) {
+ /* no socket opened, open a new socket to tx the packet and close it */
+ fd = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
+ if (fd < 0)
+ return -errno;
+
+ setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(source_port);
+ client.sin_addr.s_addr = htonl(source_ip);
+ if (bind(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
+ close(fd);
+ return -errno;
+ }
- setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(dest_port);
+ client.sin_addr.s_addr = htonl(dest_ip);
+ if (connect(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
+ close(fd);
+ return -errno;
+ }
- memset(&client, 0, sizeof(client));
- client.sin_family = AF_INET;
- client.sin_port = htons(source_port);
- client.sin_addr.s_addr = htonl(source_ip);
- if (bind(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
- close(fd);
- return -errno;
- }
+ n = write(fd, dhcp_pkt, DHCP_SIZE);
- memset(&client, 0, sizeof(client));
- client.sin_family = AF_INET;
- client.sin_port = htons(dest_port);
- client.sin_addr.s_addr = htonl(dest_ip);
- if (connect(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
close(fd);
- return -errno;
+ } else {
+ /* Using existed socket to transmit the packet */
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(dest_port);
+ client.sin_addr.s_addr = htonl(dest_ip);
+
+ n = sendto(fd, dhcp_pkt, DHCP_SIZE, MSG_DONTWAIT,
+ (struct sockaddr *) &client, sizeof(client));
}
- n = write(fd, dhcp_pkt, DHCP_SIZE);
-
- close(fd);
-
if (n < 0)
return -errno;
diff --git a/gdhcp/common.h b/gdhcp/common.h
index 75abc18..b92d214 100644
--- a/gdhcp/common.h
+++ b/gdhcp/common.h
@@ -209,7 +209,7 @@ int dhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
int dhcpv6_send_packet(int index, struct dhcpv6_packet *dhcp_pkt, int len);
int dhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
uint32_t source_ip, int source_port,
- uint32_t dest_ip, int dest_port);
+ uint32_t dest_ip, int dest_port, int fd);
int dhcp_l3_socket(int port, const char *interface, int family);
int dhcp_recv_l3_packet(struct dhcp_packet *packet, int fd);
int dhcpv6_recv_l3_packet(struct dhcpv6_packet **packet, unsigned char *buf,
--
2.7.0.rc3.207.g0ac5344
4 years, 8 months
[PATCH] core: No need to compare expression against NULL
by Saurav Babu
doc/coding-style.txt M13: Check for pointer being NULL
---
src/agent.c | 2 +-
src/dnsproxy.c | 6 +++---
src/service.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/agent.c b/src/agent.c
index 4866c8d..8f7b19b 100644
--- a/src/agent.c
+++ b/src/agent.c
@@ -586,7 +586,7 @@ static void agent_release(struct connman_agent *agent, const char *interface)
message = dbus_message_new_method_call(agent->owner, agent->path,
interface, "Release");
- if (message == NULL) {
+ if (!message) {
connman_error("Couldn't allocate D-Bus message");
return;
}
diff --git a/src/dnsproxy.c b/src/dnsproxy.c
index 06e867a..48fbef0 100644
--- a/src/dnsproxy.c
+++ b/src/dnsproxy.c
@@ -2050,21 +2050,21 @@ static int forward_dns_reply(unsigned char *reply, int reply_len, int protocol,
(char *)reply + offset, eom,
ptr, uncompressed, NS_MAXDNAME,
&uptr);
- if (ptr == NULL)
+ if (!ptr)
goto out;
ptr = uncompress(ntohs(hdr->nscount),
(char *)reply + offset, eom,
ptr, uncompressed, NS_MAXDNAME,
&uptr);
- if (ptr == NULL)
+ if (!ptr)
goto out;
ptr = uncompress(ntohs(hdr->arcount),
(char *)reply + offset, eom,
ptr, uncompressed, NS_MAXDNAME,
&uptr);
- if (ptr == NULL)
+ if (!ptr)
goto out;
/*
diff --git a/src/service.c b/src/service.c
index a93aebf..8e07337 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3653,7 +3653,7 @@ void __connman_service_set_active_session(bool enable, GSList *list)
else
active_count--;
- while (list != NULL) {
+ while (list) {
enum connman_service_type type = GPOINTER_TO_INT(list->data);
switch (type) {
--
1.9.1
4 years, 8 months
[PATCH] dhcpv6: Fix memory leak if for loop breaks
by Saurav Babu
This patch duplicates string only when it is required so that there is
no memory leak when for loop breaks.
---
src/dhcpv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/dhcpv6.c b/src/dhcpv6.c
index 97ddc26..9e21040 100644
--- a/src/dhcpv6.c
+++ b/src/dhcpv6.c
@@ -447,7 +447,6 @@ static int check_ipv6_addr_prefix(GSList *prefixes, char *address)
if (!slash)
continue;
- prefix = g_strndup(prefix, slash - prefix);
len = strtol(slash + 1, NULL, 10);
if (len < 3 || len > 128)
break;
@@ -458,6 +457,7 @@ static int check_ipv6_addr_prefix(GSList *prefixes, char *address)
left = plen % 8;
i = 16 - count;
+ prefix = g_strndup(prefix, slash - prefix);
inet_pton(AF_INET6, prefix, &addr_prefix);
inet_pton(AF_INET6, address, &addr);
--
1.9.1
4 years, 8 months
[PATCH] gdhcp: Don't send DHCPREQUEST if last assigned IP is Link Local Address
by Saurav Babu
In the following scenario:
1. connman is connected to a service and Link Local Address is obtained.
2. Disconnect the service.
3. Connect the service again.
connman tries to send DHCPREQUEST with last assigned IP which was Link
Local Address.
This patch makes connman to send DHCPDISCOVER when last assigned IP was
Link Local Address.
---
gdhcp/client.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gdhcp/client.c b/gdhcp/client.c
index 3bf8cb2..9012b38 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -2832,7 +2832,8 @@ int g_dhcp_client_start(GDHCPClient *dhcp_client, const char *last_address)
addr = 0;
} else {
addr = ntohl(inet_addr(last_address));
- if (addr == 0xFFFFFFFF) {
+ if (addr == 0xFFFFFFFF || ((addr & LINKLOCAL_ADDR) ==
+ LINKLOCAL_ADDR)) {
addr = 0;
} else if (dhcp_client->last_address != last_address) {
g_free(dhcp_client->last_address);
--
1.9.1
4 years, 8 months
Delete WiFi access points via dbus
by Pushkin Andrei
Hi. How I can delete all that connman know about AP (like rm -rf /var/lib/connman/wifi_SOME_THING) via dbus?
Andrei Pushkin
4 years, 8 months
[PATCH] gdhcp: use opened listening socket to send DHCP renew request
by Feng Wang
It fix DHCP ACK lost issue when doing DHCP renewal.
When doing DHCP renew, 2 sockets are opened. One is for
listening DHCP ACK, the other is for transmitting DHCP request
which is closed immediately after transmitting is done. But in
some cases, the socket is closed after the DHCP ACK is received.
The kernel will route the packet to the transmitting socket
because it has a better match result(dst ip/port etc). And the
packet was dropped when the socket was closed.
---
gdhcp/client.c | 6 ++++--
gdhcp/common.c | 60 +++++++++++++++++++++++++++++++++++-----------------------
gdhcp/common.h | 2 +-
3 files changed, 41 insertions(+), 27 deletions(-)
diff --git a/gdhcp/client.c b/gdhcp/client.c
index 3bf8cb2..ad587b1 100644
--- a/gdhcp/client.c
+++ b/gdhcp/client.c
@@ -502,7 +502,8 @@ static int send_request(GDHCPClient *dhcp_client)
if (dhcp_client->state == RENEWING)
return dhcp_send_kernel_packet(&packet,
dhcp_client->requested_ip, CLIENT_PORT,
- dhcp_client->server_ip, SERVER_PORT);
+ dhcp_client->server_ip, SERVER_PORT,
+ dhcp_client->listener_sockfd);
return dhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT,
INADDR_BROADCAST, SERVER_PORT,
@@ -526,7 +527,8 @@ static int send_release(GDHCPClient *dhcp_client,
dhcp_add_option_uint32(&packet, DHCP_SERVER_ID, server);
return dhcp_send_kernel_packet(&packet, ciaddr, CLIENT_PORT,
- server, SERVER_PORT);
+ server, SERVER_PORT,
+ dhcp_client->listener_sockfd);
}
static gboolean ipv4ll_probe_timeout(gpointer dhcp_data);
diff --git a/gdhcp/common.c b/gdhcp/common.c
index f3d4677..f0a9aa6 100644
--- a/gdhcp/common.c
+++ b/gdhcp/common.c
@@ -626,44 +626,56 @@ int dhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
int dhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
uint32_t source_ip, int source_port,
- uint32_t dest_ip, int dest_port)
+ uint32_t dest_ip, int dest_port, int fd)
{
struct sockaddr_in client;
- int fd, n, opt = 1;
+ int n, opt = 1;
enum {
DHCP_SIZE = sizeof(struct dhcp_packet) -
EXTEND_FOR_BUGGY_SERVERS,
};
- fd = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
- if (fd < 0)
- return -errno;
+ if (fd < 0) {
+ /* no socket opened, open a new socket to tx the packet and close it */
+ fd = socket(PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, IPPROTO_UDP);
+ if (fd < 0)
+ return -errno;
+
+ setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(source_port);
+ client.sin_addr.s_addr = htonl(source_ip);
+ if (bind(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
+ close(fd);
+ return -errno;
+ }
- setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(dest_port);
+ client.sin_addr.s_addr = htonl(dest_ip);
+ if (connect(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
+ close(fd);
+ return -errno;
+ }
- memset(&client, 0, sizeof(client));
- client.sin_family = AF_INET;
- client.sin_port = htons(source_port);
- client.sin_addr.s_addr = htonl(source_ip);
- if (bind(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
- close(fd);
- return -errno;
- }
+ n = write(fd, dhcp_pkt, DHCP_SIZE);
- memset(&client, 0, sizeof(client));
- client.sin_family = AF_INET;
- client.sin_port = htons(dest_port);
- client.sin_addr.s_addr = htonl(dest_ip);
- if (connect(fd, (struct sockaddr *) &client, sizeof(client)) < 0) {
close(fd);
- return -errno;
+ } else {
+ /* Using existed socket to transmit the packet */
+ memset(&client, 0, sizeof(client));
+ client.sin_family = AF_INET;
+ client.sin_port = htons(dest_port);
+ client.sin_addr.s_addr = htonl(dest_ip);
+
+ n = sendto(fd, dhcp_pkt, DHCP_SIZE, MSG_DONTWAIT,
+ (struct sockaddr *) &client, sizeof(client));
}
- n = write(fd, dhcp_pkt, DHCP_SIZE);
-
- close(fd);
-
if (n < 0)
return -errno;
diff --git a/gdhcp/common.h b/gdhcp/common.h
index 75abc18..b92d214 100644
--- a/gdhcp/common.h
+++ b/gdhcp/common.h
@@ -209,7 +209,7 @@ int dhcp_send_raw_packet(struct dhcp_packet *dhcp_pkt,
int dhcpv6_send_packet(int index, struct dhcpv6_packet *dhcp_pkt, int len);
int dhcp_send_kernel_packet(struct dhcp_packet *dhcp_pkt,
uint32_t source_ip, int source_port,
- uint32_t dest_ip, int dest_port);
+ uint32_t dest_ip, int dest_port, int fd);
int dhcp_l3_socket(int port, const char *interface, int family);
int dhcp_recv_l3_packet(struct dhcp_packet *packet, int fd);
int dhcpv6_recv_l3_packet(struct dhcpv6_packet **packet, unsigned char *buf,
--
2.7.0.rc3.207.g0ac5344
4 years, 8 months
