DNS issue in connman
Hi All, I'm facing an issue in Connman when a client finds the 'Truncated flag' enabled in the DNS response and tries to resend DNS request over TCP. I've checked recent DNS commits which happened in Connman, but those changes are not helping completely. Please let me know if someone has come across this kind of issue. Thanks in advance. Regards, Naveen ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/3] supplicant: Tie P2P flush D-Bus call to the interface
As the callback for the call accesses GSupplicantInterface structure, tie the pending call to the interface pointer so that it gets cancelled if the interface is removed. --- gsupplicant/supplicant.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 2674298..0b42ce8 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -1964,7 +1964,7 @@ static void interface_added(DBusMessageIter *iter, void *user_data) supplicant_dbus_method_call(path, SUPPLICANT_INTERFACE .Interface.P2PDevice, Flush, - NULL, interface_p2p_flush, interface, NULL); + NULL, interface_p2p_flush, interface, interface); dbus_message_iter_next(iter); if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INVALID) { -- 1.8.5.3 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 3/3] wifi: cancel pending supplicant operations when removing
If there are pending D-Bus calls made inside supplicant code when wifi_remove() frees the wifi struct, wifi plugin callback functions may attempt to use already released memory when the calls complete. Fixed by cancelling any outstanding supplicant calls in wifi_remove(). --- gsupplicant/gsupplicant.h | 2 ++ gsupplicant/supplicant.c | 15 +-- plugins/wifi.c| 2 ++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/gsupplicant/gsupplicant.h b/gsupplicant/gsupplicant.h index a5ec405..7a3c843 100644 --- a/gsupplicant/gsupplicant.h +++ b/gsupplicant/gsupplicant.h @@ -172,6 +172,8 @@ typedef void (*GSupplicantInterfaceCallback) (int result, GSupplicantInterface *interface, void *user_data); +void g_supplicant_interface_cancel(GSupplicantInterface *interface); + int g_supplicant_interface_create(const char *ifname, const char *driver, const char *bridge, GSupplicantInterfaceCallback callback, diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 7b0f4d4..ea68433 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -1990,9 +1990,7 @@ static void interface_removed(DBusMessageIter *iter, void *user_data) return; interface = g_hash_table_lookup(interface_table, path); - SUPPLICANT_DBG(Cancelling any pending DBus calls); - supplicant_dbus_method_call_cancel_all(interface); - supplicant_dbus_property_call_cancel_all(interface); + g_supplicant_interface_cancel(interface); g_hash_table_remove(interface_table, path); } @@ -2582,6 +2580,13 @@ static DBusHandlerResult g_supplicant_filter(DBusConnection *conn, return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } +void g_supplicant_interface_cancel(GSupplicantInterface *interface) +{ + SUPPLICANT_DBG(Cancelling any pending DBus calls); + supplicant_dbus_method_call_cancel_all(interface); + supplicant_dbus_property_call_cancel_all(interface); +} + struct supplicant_regdom { GSupplicantCountryCallback callback; const char *alpha2; @@ -2980,9 +2985,7 @@ int g_supplicant_interface_remove(GSupplicantInterface *interface, if (!system_available) return -EFAULT; - SUPPLICANT_DBG(Cancelling any pending DBus calls); - supplicant_dbus_method_call_cancel_all(interface); - supplicant_dbus_property_call_cancel_all(interface); + g_supplicant_interface_cancel(interface); data = dbus_malloc0(sizeof(*data)); if (!data) diff --git a/plugins/wifi.c b/plugins/wifi.c index ef4dd95..aaa5b00 100644 --- a/plugins/wifi.c +++ b/plugins/wifi.c @@ -313,6 +313,8 @@ static void wifi_remove(struct connman_device *device) g_supplicant_interface_set_data(wifi-interface, NULL); + g_supplicant_interface_cancel(wifi-interface); + if (wifi-scan_params) g_supplicant_free_scan_params(wifi-scan_params); -- 1.8.5.3 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/3] supplicant: rewrite pending DBus call handling for properties
Property-related D-Bus calls may be pending replies when the related GSupplicantInterface structure is cleaned up. This results in an attempt to access already freed memory when calls complete. Fixed by keeping a list of pending property calls, and canceling the calls related to an interface when the interface is removed. This is a similar change to the one made earlier for pending method calls. --- gsupplicant/dbus.c | 140 ++- gsupplicant/dbus.h | 12 ++-- gsupplicant/supplicant.c | 33 ++- 3 files changed, 116 insertions(+), 69 deletions(-) diff --git a/gsupplicant/dbus.c b/gsupplicant/dbus.c index 5b15bcd..130306e 100644 --- a/gsupplicant/dbus.c +++ b/gsupplicant/dbus.c @@ -59,10 +59,35 @@ static int find_method_call_by_caller(gconstpointer a, gconstpointer b) return method_call-caller != caller; } +static GSList *property_calls; + +struct property_call_data { + gpointer caller; + DBusPendingCall *pending_call; + supplicant_dbus_property_function function; + void *user_data; +}; + +static void property_call_free(void *pointer) +{ + struct property_call_data *property_call = pointer; + property_calls = g_slist_remove(property_calls, property_call); + g_free(property_call); +} + +static int find_property_call_by_caller(gconstpointer a, gconstpointer b) +{ + const struct property_call_data *property_call = a; + gconstpointer caller = b; + + return property_call-caller != caller; +} + void supplicant_dbus_setup(DBusConnection *conn) { connection = conn; method_calls = NULL; + property_calls = NULL; } void supplicant_dbus_array_foreach(DBusMessageIter *iter, @@ -124,14 +149,27 @@ void supplicant_dbus_property_foreach(DBusMessageIter *iter, } } -struct property_get_data { - supplicant_dbus_property_function function; - void *user_data; -}; +void supplicant_dbus_property_call_cancel_all(gpointer caller) +{ + while (property_calls) { + struct property_call_data *property_call; + GSList *elem = g_slist_find_custom(property_calls, caller, + find_property_call_by_caller); + if (!elem) + break; + + property_call = elem-data; + property_calls = g_slist_delete_link(property_calls, elem); + + dbus_pending_call_cancel(property_call-pending_call); + + dbus_pending_call_unref(property_call-pending_call); + } +} static void property_get_all_reply(DBusPendingCall *call, void *user_data) { - struct property_get_data *data = user_data; + struct property_call_data *property_call = user_data; DBusMessage *reply; DBusMessageIter iter; @@ -143,11 +181,11 @@ static void property_get_all_reply(DBusPendingCall *call, void *user_data) if (!dbus_message_iter_init(reply, iter)) goto done; - supplicant_dbus_property_foreach(iter, data-function, - data-user_data); + supplicant_dbus_property_foreach(iter, property_call-function, + property_call-user_data); - if (data-function) - data-function(NULL, NULL, data-user_data); + if (property_call-function) + property_call-function(NULL, NULL, property_call-user_data); done: dbus_message_unref(reply); @@ -157,9 +195,9 @@ done: int supplicant_dbus_property_get_all(const char *path, const char *interface, supplicant_dbus_property_function function, - void *user_data) + void *user_data, gpointer caller) { - struct property_get_data *data; + struct property_call_data *property_call = NULL; DBusMessage *message; DBusPendingCall *call; @@ -169,14 +207,14 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!path || !interface) return -EINVAL; - data = dbus_malloc0(sizeof(*data)); - if (!data) + property_call = g_try_new0(struct property_call_data, 1); + if (!property_call) return -ENOMEM; message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path, DBUS_INTERFACE_PROPERTIES, GetAll); if (!message) { - dbus_free(data); + g_free(property_call); return -ENOMEM; } @@ -187,21 +225,23 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!dbus_connection_send_with_reply(connection, message, call, TIMEOUT)) { dbus_message_unref(message); -
[PATCH 3/3] wifi: cancel pending supplicant operations when removing
If there are pending D-Bus calls made inside supplicant code when wifi_remove() frees the wifi struct, wifi plugin callback functions may attempt to use already released memory when the calls complete. Fixed by cancelling any outstanding supplicant calls in wifi_remove(). --- gsupplicant/gsupplicant.h | 2 ++ gsupplicant/supplicant.c | 15 +-- plugins/wifi.c| 2 ++ 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/gsupplicant/gsupplicant.h b/gsupplicant/gsupplicant.h index a5ec405..7a3c843 100644 --- a/gsupplicant/gsupplicant.h +++ b/gsupplicant/gsupplicant.h @@ -172,6 +172,8 @@ typedef void (*GSupplicantInterfaceCallback) (int result, GSupplicantInterface *interface, void *user_data); +void g_supplicant_interface_cancel(GSupplicantInterface *interface); + int g_supplicant_interface_create(const char *ifname, const char *driver, const char *bridge, GSupplicantInterfaceCallback callback, diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 7b0f4d4..ea68433 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -1990,9 +1990,7 @@ static void interface_removed(DBusMessageIter *iter, void *user_data) return; interface = g_hash_table_lookup(interface_table, path); - SUPPLICANT_DBG(Cancelling any pending DBus calls); - supplicant_dbus_method_call_cancel_all(interface); - supplicant_dbus_property_call_cancel_all(interface); + g_supplicant_interface_cancel(interface); g_hash_table_remove(interface_table, path); } @@ -2582,6 +2580,13 @@ static DBusHandlerResult g_supplicant_filter(DBusConnection *conn, return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; } +void g_supplicant_interface_cancel(GSupplicantInterface *interface) +{ + SUPPLICANT_DBG(Cancelling any pending DBus calls); + supplicant_dbus_method_call_cancel_all(interface); + supplicant_dbus_property_call_cancel_all(interface); +} + struct supplicant_regdom { GSupplicantCountryCallback callback; const char *alpha2; @@ -2980,9 +2985,7 @@ int g_supplicant_interface_remove(GSupplicantInterface *interface, if (!system_available) return -EFAULT; - SUPPLICANT_DBG(Cancelling any pending DBus calls); - supplicant_dbus_method_call_cancel_all(interface); - supplicant_dbus_property_call_cancel_all(interface); + g_supplicant_interface_cancel(interface); data = dbus_malloc0(sizeof(*data)); if (!data) diff --git a/plugins/wifi.c b/plugins/wifi.c index ef4dd95..aaa5b00 100644 --- a/plugins/wifi.c +++ b/plugins/wifi.c @@ -313,6 +313,8 @@ static void wifi_remove(struct connman_device *device) g_supplicant_interface_set_data(wifi-interface, NULL); + g_supplicant_interface_cancel(wifi-interface); + if (wifi-scan_params) g_supplicant_free_scan_params(wifi-scan_params); -- 1.8.5.3 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/3] supplicant: rewrite pending DBus call handling for properties
Property-related D-Bus calls may be pending replies when the related GSupplicantInterface structure is cleaned up. This results in an attempt to access already freed memory when calls complete. Fixed by keeping a list of pending property calls, and canceling the calls related to an interface when the interface is removed. This is a similar change to the one made earlier for pending method calls. --- gsupplicant/dbus.c | 140 ++- gsupplicant/dbus.h | 12 ++-- gsupplicant/supplicant.c | 33 ++- 3 files changed, 116 insertions(+), 69 deletions(-) diff --git a/gsupplicant/dbus.c b/gsupplicant/dbus.c index 5b15bcd..130306e 100644 --- a/gsupplicant/dbus.c +++ b/gsupplicant/dbus.c @@ -59,10 +59,35 @@ static int find_method_call_by_caller(gconstpointer a, gconstpointer b) return method_call-caller != caller; } +static GSList *property_calls; + +struct property_call_data { + gpointer caller; + DBusPendingCall *pending_call; + supplicant_dbus_property_function function; + void *user_data; +}; + +static void property_call_free(void *pointer) +{ + struct property_call_data *property_call = pointer; + property_calls = g_slist_remove(property_calls, property_call); + g_free(property_call); +} + +static int find_property_call_by_caller(gconstpointer a, gconstpointer b) +{ + const struct property_call_data *property_call = a; + gconstpointer caller = b; + + return property_call-caller != caller; +} + void supplicant_dbus_setup(DBusConnection *conn) { connection = conn; method_calls = NULL; + property_calls = NULL; } void supplicant_dbus_array_foreach(DBusMessageIter *iter, @@ -124,14 +149,27 @@ void supplicant_dbus_property_foreach(DBusMessageIter *iter, } } -struct property_get_data { - supplicant_dbus_property_function function; - void *user_data; -}; +void supplicant_dbus_property_call_cancel_all(gpointer caller) +{ + while (property_calls) { + struct property_call_data *property_call; + GSList *elem = g_slist_find_custom(property_calls, caller, + find_property_call_by_caller); + if (!elem) + break; + + property_call = elem-data; + property_calls = g_slist_delete_link(property_calls, elem); + + dbus_pending_call_cancel(property_call-pending_call); + + dbus_pending_call_unref(property_call-pending_call); + } +} static void property_get_all_reply(DBusPendingCall *call, void *user_data) { - struct property_get_data *data = user_data; + struct property_call_data *property_call = user_data; DBusMessage *reply; DBusMessageIter iter; @@ -143,11 +181,11 @@ static void property_get_all_reply(DBusPendingCall *call, void *user_data) if (!dbus_message_iter_init(reply, iter)) goto done; - supplicant_dbus_property_foreach(iter, data-function, - data-user_data); + supplicant_dbus_property_foreach(iter, property_call-function, + property_call-user_data); - if (data-function) - data-function(NULL, NULL, data-user_data); + if (property_call-function) + property_call-function(NULL, NULL, property_call-user_data); done: dbus_message_unref(reply); @@ -157,9 +195,9 @@ done: int supplicant_dbus_property_get_all(const char *path, const char *interface, supplicant_dbus_property_function function, - void *user_data) + void *user_data, gpointer caller) { - struct property_get_data *data; + struct property_call_data *property_call = NULL; DBusMessage *message; DBusPendingCall *call; @@ -169,14 +207,14 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!path || !interface) return -EINVAL; - data = dbus_malloc0(sizeof(*data)); - if (!data) + property_call = g_try_new0(struct property_call_data, 1); + if (!property_call) return -ENOMEM; message = dbus_message_new_method_call(SUPPLICANT_SERVICE, path, DBUS_INTERFACE_PROPERTIES, GetAll); if (!message) { - dbus_free(data); + g_free(property_call); return -ENOMEM; } @@ -187,21 +225,23 @@ int supplicant_dbus_property_get_all(const char *path, const char *interface, if (!dbus_connection_send_with_reply(connection, message, call, TIMEOUT)) { dbus_message_unref(message); -
[PATCH 1/3] supplicant: Tie P2P flush D-Bus call to the interface
As the callback for the call accesses GSupplicantInterface structure, tie the pending call to the interface pointer so that it gets cancelled if the interface is removed. --- gsupplicant/supplicant.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index 2674298..0b42ce8 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -1964,7 +1964,7 @@ static void interface_added(DBusMessageIter *iter, void *user_data) supplicant_dbus_method_call(path, SUPPLICANT_INTERFACE .Interface.P2PDevice, Flush, - NULL, interface_p2p_flush, interface, NULL); + NULL, interface_p2p_flush, interface, interface); dbus_message_iter_next(iter); if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_INVALID) { -- 1.8.5.3 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: openvpn: client ip doesn't change
On Wed, 2014-06-18 at 18:05 +0300, Yevhen Kyriukha wrote: Hello, What about this issue? Has anyone started to work on it already? Seems to be important for those who use VPN with dynamic IP address assignment. I was investigating the code to fix this issue by myself but not quite understand how everything is working. What I found is that the problem is in VPN plugin for connman. Connman simply doesn't update IP address when reconnects to VPN. Could you provide the log output with 'connman-vpnd -n -d' for this? Thanks, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: DNS issue in connman
Hi, On Thu, 2014-06-19 at 11:53 +0530, Naveen Hiregoudar wrote: Hi All, I'm facing an issue in Connman when a client finds the 'Truncated flag' enabled in the DNS response and tries to resend DNS request over TCP. I've checked recent DNS commits which happened in Connman, but those changes are not helping completely. Please let me know if someone has come across this kind of issue. Please describe your problem in a bit more detail. The log output of the failing DNS lookup would be important to have, as would the corresponding tcpdump as the exact content of the UDP or TCP DNS packets would be needed to debug the issue. Is this the first lookup or a subsequent one where the result is already cached by ConnMan? Is this really going over TCP, as that's used only if the request does not fit into an UDP one? Cheers, Patrik ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: DNS issue in connman
Hi Patrick, Please find below the connman logs for the test program which is provided below: Error logs in case of TCP: == 06/19 18:02:42 tcp_listener_event condition 0x01 channel 0x2046488 ifdata 0x2045c98 family 2 06/19 18:02:42 tcp_listener_event client 16 accepted 06/19 18:02:42 tcp_listener_event client 16 created 0x204e458 06/19 18:02:42 tcp_listener_event client 16 sent 28 bytes but expecting 1595 pending 1567 06/19 18:02:45 client_timeout client 16 timeout pending 28 bytes 06/19 18:02:45 client_reset client 16 closing And in case of UDP below are the logs: == 06/19 17:56:56 udp_listener_event Received 41 bytes (id 0xa52e) 06/19 17:56:56 parse_request id 0xa52e qr 0 opcode 0 qdcount 1 arcount 0 06/19 17:56:56 parse_request query gld.push.samsungosp.com. 06/19 17:56:56 resolv server 192.168.1.1 enabled 1 06/19 17:56:56 cache_enforce_validity cache timeout gldpush samsungospcom type A 06/19 17:56:56 cache_check_validity cache entry missing gldpush samsungospcom type A 06/19 17:56:56 udp_listener_event req 0x10dda48 dstid 0x4fe0 altid 0x22f4 06/19 17:57:01 udp_listener_event Received 41 bytes (id 0xa52e) 06/19 17:57:01 parse_request id 0xa52e qr 0 opcode 0 qdcount 1 arcount 0 06/19 17:57:01 parse_request query gld.push.samsungosp.com. 06/19 17:57:01 resolv server 192.168.1.1 enabled 1 06/19 17:57:01 udp_listener_event req 0x10ddb88 dstid 0x673c altid 0xdd38 06/19 17:57:01 forward_dns_reply Received 224 bytes (id 0x673c) 06/19 17:57:01 forward_dns_reply req 0x10ddb88 dstid 0x673c altid 0xdd38 rcode 0 06/19 17:57:01 cache_update offset 0 hdr 0xbeacab1c msg 0xbeacab1c rcode 0 06/19 17:57:01 parse_response qr 1 qdcount 1 06/19 17:57:01 cache_update cache 4 new question gldpush samsungospcom type 1 ttl 44 size 243 packet 171 dns len 171 06/19 17:57:01 forward_dns_reply proto 17 sent 224 bytes to 8 06/19 17:57:01 udp_listener_event Received 41 bytes (id 0xe54a) 06/19 17:57:01 parse_request id 0xe54a qr 0 opcode 0 qdcount 1 arcount 0 06/19 17:57:01 parse_request query gld.push.samsungosp.com. 06/19 17:57:01 resolv server 192.168.1.1 enabled 1 06/19 17:57:01 ns_resolv cache hit gld.push.samsungosp.com. type A 06/19 17:57:01 send_cached_response sk 8 id 0xe54a answers 8 ptr 0x10e6f6a length 169 dns 169 06/19 17:57:26 request_timeout id 0xa52e 06/19 18:01:17 inotify_data 06/19 18:01:17 inotify_data bytes read 48 And here is the test program to simulate the issue: = .. s = socket(AF_INET , SOCK_STREAM , IPPROTO_TCP); dest.sin_family = AF_INET; dest.sin_port = htons(53); dest.sin_addr.s_addr = inet_addr(127.0.0.1); .. qname =(unsigned char*)buf[sizeof(struct DNS_HEADER)]; /* This will convert www.google.com to 3www6google3com */ ChangetoDnsNameFormat(qname , host); printf(\n qname=%s,qname); qinfo =(struct QUESTION*)buf[sizeof(struct DNS_HEADER) + (strlen((const char*)qname) + 1)]; qinfo-qtype = htons(T_PTR); qinfo-qclass = htons(1); //its internet (lol) if (connect(s, (const struct sockaddr *)dest, sizeof(struct sockaddr_in)) 0) { perror(connect failed); } printf(\nSending Packet...); if (send(s, (char*)buf, sizeof(struct DNS_HEADER) + (strlen((const char*)qname)+1) + sizeof(struct QUESTION), 0) 0) { perror(send failed); } printf(Done); printf(\nReceiving answer...); if (recv (s,(char*)buf , sizeof(buf) , 0) 0) { perror(recvfrom failed); } = If we change the above program to UDP, it works fine. The data itself is not processed to get the tcpdump in this case so unable to get any meaningful tcpdump. And this is the case of first lookup which fails. Please let me know if you need any further information. Regards, Naveen -- Message: 4 Date: Thu, 19 Jun 2014 15:05:16 +0300 From: Patrik Flykt patrik.fl...@linux.intel.com To: connman@connman.net Subject: Re: DNS issue in connman Message-ID: 1403179516.30880.14.camel@pflykt-mobl1 Content-Type: text/plain; charset=UTF-8 Hi, On Thu, 2014-06-19 at 11:53 +0530, Naveen Hiregoudar wrote: Hi All, I'm facing an issue in Connman when a client finds the 'Truncated flag' enabled in the DNS response and tries to resend DNS request over TCP. I've checked recent DNS commits which happened in Connman, but those changes are not helping completely. Please let me know if someone has come across this kind of issue. Please describe your problem in a bit more detail. The log output of the failing DNS lookup would be important to have, as would the corresponding tcpdump as the exact content of the UDP or TCP DNS packets would be needed to debug the issue. Is this the first lookup or a subsequent one where the result is already cached by ConnMan? Is this really going over TCP, as that's used only if the request does not fit into