[PATCH] dnsproxy: Check whether cache cleanup needs to be scheduled
server_create_socket() may fail for some reason before the cache hash table is created. In this case destroy_server() should not schedule a timeout for deallocating the cache; if it does, cache_refcount may become negative and an attempt to create the cache later on will not work properly, leading to a null pointer crash when trying to use the cache. --- src/dnsproxy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dnsproxy.c b/src/dnsproxy.c index 741cd45..bdd7fd5 100644 --- a/src/dnsproxy.c +++ b/src/dnsproxy.c @@ -2204,7 +2204,8 @@ static void destroy_server(struct server_data *server) * without any good reason. The small delay allows the new RDNSS to * create a new DNS server instance and the refcount does not go to 0. */ - g_timeout_add_seconds(3, try_remove_cache, NULL); + if (cache) + g_timeout_add_seconds(3, try_remove_cache, NULL); g_free(server); } -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/2] unit: Add unit test for dnsproxy cache cleanup check
Unit test for checking that a nonexistent dnsproxy cache is not cleaned up. --- Makefile.am | 9 ++- unit/test-dnsproxy.c | 160 +++ 2 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 unit/test-dnsproxy.c diff --git a/Makefile.am b/Makefile.am index a7f3ed3..b651d92 100644 --- a/Makefile.am +++ b/Makefile.am @@ -254,7 +254,8 @@ client_connmanctl_LDADD = gdbus/libgdbus-internal.la @DBUS_LIBS@ @GLIB_LIBS@ \ -lreadline -ldl endif -noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool +noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \ + unit/test-dnsproxy unit_test_pbkdf2_sha1_SOURCES = unit/test-pbkdf2-sha1.c \ src/shared/sha1.h src/shared/sha1.c @@ -269,7 +270,11 @@ unit_test_ippool_SOURCES = src/log.c src/dbus.c src/error.c \ unit_test_ippool_LDADD = gdbus/libgdbus-internal.la \ @GLIB_LIBS@ @DBUS_LIBS@ -ldl -TESTS = unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool +unit_test_dnsproxy_SOURCES = unit/test-dnsproxy.c src/log.c +unit_test_dnsproxy_LDADD = @GLIB_LIBS@ -ldl + +TESTS = unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \ + unit/test-dnsproxy if WISPR noinst_PROGRAMS += tools/wispr diff --git a/unit/test-dnsproxy.c b/unit/test-dnsproxy.c new file mode 100644 index 000..6435c2b --- /dev/null +++ b/unit/test-dnsproxy.c @@ -0,0 +1,160 @@ +/* + * + * Connection Manager + * + * Copyright (C) 2014 Jolla Ltd. All rights reserved. + * Contact: Hannu Mallat hannu.mal...@jollamobile.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + */ + +/* Include source file to access static variables easily */ +#include src/dnsproxy.c + +static GMainLoop *main_loop = NULL; + +/* Stub getaddrinfo() to return test data */ +int getaddrinfo(const char *node, const char *service, + const struct addrinfo *hints, + struct addrinfo **res) +{ + struct addrinfo *ai = g_new0(struct addrinfo, 1); + ai-ai_socktype = hints-ai_socktype; + ai-ai_protocol = hints-ai_protocol; + if (hints-ai_family == AF_INET6) { + struct sockaddr_in6 *in6 = g_new0(struct sockaddr_in6, 1); + in6-sin6_family = AF_INET6; + in6-sin6_port = htons(53); + memcpy(in6-sin6_addr.s6_addr, 0123456789abcdef, 16); + + ai-ai_family = AF_INET6; + ai-ai_addrlen = sizeof(struct sockaddr_in6); + ai-ai_addr = (struct sockaddr *)in6; + } else { + struct sockaddr_in *in = g_new0(struct sockaddr_in, 1); + in-sin_family = AF_INET; + in-sin_port = htons(53); + in-sin_addr.s_addr = htonl(0x12345678); + + ai-ai_family = AF_INET6; + ai-ai_addrlen = sizeof(struct sockaddr_in); + ai-ai_addr = (struct sockaddr *)in; + } + ai-ai_canonname = g_strdup(node); + ai-ai_next = NULL; + *res = ai; + + return 0; +} + +void freeaddrinfo(struct addrinfo *res) +{ + if (res) { + if (res-ai_addr) { + g_free(res-ai_addr); + } + if (res-ai_canonname) { + g_free(res-ai_canonname); + } + g_free(res); + } +} + +/* Stub socket() that always fails */ +int socket(int domain, int type, int protocol) +{ + return -1; +} + +int connman_inet_ifindex(const char *name) +{ + return -1; +} + +int __connman_service_get_index(struct connman_service *service) +{ + return -1; +} + +bool __connman_service_index_is_default(int index) +{ + return FALSE; +} + +bool __connman_service_index_is_split_routing(int index) +{ + return FALSE; +} + +int __connman_resolvfile_append(int index, const char *domain, const char *server) +{ + return -1; +} + +int __connman_resolvfile_remove(int index, const char *domain, const char *server) +{ + return -1; +} + +int connman_notifier_register(struct connman_notifier *notifier) +{ + return 0; +} + +void connman_notifier_unregister(struct connman_notifier *notifier) +{ +} + +static gboolean
[PATCH 1/2] dnsproxy: Check whether cache cleanup needs to be scheduled
server_create_socket() may fail for some reason before the cache hash table is created. In this case destroy_server() should not schedule a timeout for deallocating the cache; if it does, cache_refcount may become negative and an attempt to create the cache later on will not work properly, leading to a null pointer crash when trying to use the cache. --- src/dnsproxy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dnsproxy.c b/src/dnsproxy.c index 741cd45..bdd7fd5 100644 --- a/src/dnsproxy.c +++ b/src/dnsproxy.c @@ -2204,7 +2204,8 @@ static void destroy_server(struct server_data *server) * without any good reason. The small delay allows the new RDNSS to * create a new DNS server instance and the refcount does not go to 0. */ - g_timeout_add_seconds(3, try_remove_cache, NULL); + if (cache) + g_timeout_add_seconds(3, try_remove_cache, NULL); g_free(server); } -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 0/2] dnsproxy: Check whether cache cleanup needs to be scheduled
Re-sending the previous patch, this time with a unit test that checks a cleanup timeout isn't scheduled for nonexistent caches. It takes a few seconds to run the test, as it waits for the cleanup timeout to complete, but I didn't want to change the source under test just to speed up the test execution. Hannu Mallat (2): dnsproxy: Check whether cache cleanup needs to be scheduled unit: Add unit test for dnsproxy cache cleanup check Makefile.am | 9 ++- src/dnsproxy.c | 3 +- unit/test-dnsproxy.c | 160 +++ 3 files changed, 169 insertions(+), 3 deletions(-) create mode 100644 unit/test-dnsproxy.c -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/2] unit: Add unit test for dnsproxy cache cleanup check
Unit test for checking that a nonexistent dnsproxy cache is not cleaned up. --- Makefile.am | 9 ++- unit/test-dnsproxy.c | 192 +++ 2 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 unit/test-dnsproxy.c diff --git a/Makefile.am b/Makefile.am index a7f3ed3..e0b44b9 100644 --- a/Makefile.am +++ b/Makefile.am @@ -254,7 +254,8 @@ client_connmanctl_LDADD = gdbus/libgdbus-internal.la @DBUS_LIBS@ @GLIB_LIBS@ \ -lreadline -ldl endif -noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool +noinst_PROGRAMS += unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \ + unit/test-dnsproxy unit_test_pbkdf2_sha1_SOURCES = unit/test-pbkdf2-sha1.c \ src/shared/sha1.h src/shared/sha1.c @@ -269,7 +270,11 @@ unit_test_ippool_SOURCES = src/log.c src/dbus.c src/error.c \ unit_test_ippool_LDADD = gdbus/libgdbus-internal.la \ @GLIB_LIBS@ @DBUS_LIBS@ -ldl -TESTS = unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool +unit_test_dnsproxy_SOURCES = unit/test-dnsproxy.c src/log.c +unit_test_dnsproxy_LDADD = @GLIB_LIBS@ -lresolv -ldl + +TESTS = unit/test-pbkdf2-sha1 unit/test-prf-sha1 unit/test-ippool \ + unit/test-dnsproxy if WISPR noinst_PROGRAMS += tools/wispr diff --git a/unit/test-dnsproxy.c b/unit/test-dnsproxy.c new file mode 100644 index 000..ebe9587 --- /dev/null +++ b/unit/test-dnsproxy.c @@ -0,0 +1,192 @@ +/* + * + * Connection Manager + * + * Copyright (C) 2014 Jolla Ltd. All rights reserved. + * Contact: Hannu Mallat hannu.mal...@jollamobile.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + */ + +/* Include source file to access static variables easily */ +#include src/dnsproxy.c + +static GMainLoop *main_loop = NULL; + +/* Stub getaddrinfo() to return test data */ +int getaddrinfo(const char *node, const char *service, + const struct addrinfo *hints, + struct addrinfo **res) +{ + struct addrinfo *ai = g_new0(struct addrinfo, 1); + ai-ai_socktype = hints-ai_socktype; + ai-ai_protocol = hints-ai_protocol; + if (hints-ai_family == AF_INET6) { + struct sockaddr_in6 *in6 = g_new0(struct sockaddr_in6, 1); + in6-sin6_family = AF_INET6; + in6-sin6_port = htons(53); + memcpy(in6-sin6_addr.s6_addr, 0123456789abcdef, 16); + + ai-ai_family = AF_INET6; + ai-ai_addrlen = sizeof(struct sockaddr_in6); + ai-ai_addr = (struct sockaddr *)in6; + } else { + struct sockaddr_in *in = g_new0(struct sockaddr_in, 1); + in-sin_family = AF_INET; + in-sin_port = htons(53); + in-sin_addr.s_addr = htonl(0x12345678); + + ai-ai_family = AF_INET6; + ai-ai_addrlen = sizeof(struct sockaddr_in); + ai-ai_addr = (struct sockaddr *)in; + } + ai-ai_canonname = g_strdup(node); + ai-ai_next = NULL; + *res = ai; + + return 0; +} + +void freeaddrinfo(struct addrinfo *res) +{ + if (res) { + if (res-ai_addr) { + g_free(res-ai_addr); + } + if (res-ai_canonname) { + g_free(res-ai_canonname); + } + g_free(res); + } +} + +/* Stub socket() that always fails */ +int socket(int domain, int type, int protocol) +{ + return -1; +} + +GResolv *g_resolv_new(int index) +{ + return NULL; +} + +bool g_resolv_set_address_family(GResolv *resolv, int family) +{ + return FALSE; +} + +bool g_resolv_add_nameserver(GResolv *resolv, const char *address, + uint16_t port, unsigned long flags) +{ + return FALSE; +} + +guint g_resolv_lookup_hostname(GResolv *resolv, const char *hostname, + GResolvResultFunc func, gpointer user_data) +{ + return 0; +} + +char *connman_inet_ifname(int index) +{ + return NULL; +} + +int connman_inet_ifindex(const char *name) +{ + return -1; +} + +int __connman_inet_get_interface_address(int index, int family, void *address) +{ + return -1; +} + +int
[PATCH 1/2] dnsproxy: Check whether cache cleanup needs to be scheduled
server_create_socket() may fail for some reason before the cache hash table is created. In this case destroy_server() should not schedule a timeout for deallocating the cache; if it does, cache_refcount may become negative and an attempt to create the cache later on will not work properly, leading to a null pointer crash when trying to use the cache. --- src/dnsproxy.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/dnsproxy.c b/src/dnsproxy.c index 741cd45..bdd7fd5 100644 --- a/src/dnsproxy.c +++ b/src/dnsproxy.c @@ -2204,7 +2204,8 @@ static void destroy_server(struct server_data *server) * without any good reason. The small delay allows the new RDNSS to * create a new DNS server instance and the refcount does not go to 0. */ - g_timeout_add_seconds(3, try_remove_cache, NULL); + if (cache) + g_timeout_add_seconds(3, try_remove_cache, NULL); g_free(server); } -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 0/2] dnsproxy: Check whether cache cleanup needs to be scheduled
One more reworking: unit test needs a few more stubs when compiling without optimization. Hannu Mallat (2): dnsproxy: Check whether cache cleanup needs to be scheduled unit: Add unit test for dnsproxy cache cleanup check Makefile.am | 9 ++- src/dnsproxy.c | 3 +- unit/test-dnsproxy.c | 192 +++ 3 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 unit/test-dnsproxy.c -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/3] wifi: Check peer_service user_data before invoking cb
A check was added, due to the occurrence of peer service registration without callback. Such scenarios takes place, when a previous peer service was registered with a proper callback, on a distinct interface. --- plugins/wifi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/wifi.c b/plugins/wifi.c index f16c3fe..45d88b8 100644 --- a/plugins/wifi.c +++ b/plugins/wifi.c @@ -376,10 +376,10 @@ static void register_peer_service_cb(int result, wifi-servicing++; } - if (reg_data-callback) + if (reg_data reg_data-callback) { reg_data-callback(result, reg_data-user_data); - - g_free(reg_data); + g_free(reg_data); + } } static GSupplicantP2PServiceParams *fill_in_peer_service_params( -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 0/3] Peer service registration fixes
This set aims to solve observed issues during peer service register and unregister scenarios. Eduardo Abinader (3): gsupplicant: Check interface on interface_has_p2p wifi: Check peer_service user_data before invoking cb manager: Do not recurse one more level for upnp basic type gsupplicant/supplicant.c | 3 +++ plugins/wifi.c | 6 +++--- src/manager.c| 7 --- 3 files changed, 10 insertions(+), 6 deletions(-) -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/3] gsupplicant: Check interface on interface_has_p2p
Solves a segmentation fault occuring on peer service registration, when a wifi plugin did not have an associated interface while looping in peer service registration. --- gsupplicant/supplicant.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index fd16caf..b5e3930 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -3181,6 +3181,9 @@ int g_supplicant_interface_set_country(GSupplicantInterface *interface, bool g_supplicant_interface_has_p2p(GSupplicantInterface *interface) { + if (!interface) + return false; + return interface-p2p_support; } -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 0/3] Peer service registration fixes
Thanks Eduardo, ACK from me. Tomasz ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 0/3] Peer service registration fixes
Thanks, Tomasz. But I just mixed things here: patch 3/3 is actually a separate patch I sent yesterday to ML. Patch 3/3 is actually another one regarding peer_driver unregister. Resending a v2. On Wed, Sep 17, 2014 at 8:01 AM, Tomasz Bursztyka tomasz.burszt...@linux.intel.com wrote: Thanks Eduardo, ACK from me. Tomasz ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/4] gdhcp: Transaction id was not set when receiving L2 packet
The transaction id was always printed as 0x in debug print. --- gdhcp/client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gdhcp/client.c b/gdhcp/client.c index 88c0419..a031501 100644 --- a/gdhcp/client.c +++ b/gdhcp/client.c @@ -2257,6 +2257,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, if (dhcp_client-listen_mode == L2) { re = dhcp_recv_l2_packet(packet, dhcp_client-listener_sockfd); + xid = packet.xid; } else if (dhcp_client-listen_mode == L3) { if (dhcp_client-type == G_DHCP_IPV6) { re = dhcpv6_recv_l3_packet(packet6, buf, sizeof(buf), -- 1.8.3.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/4] gdhcp: Transaction id is in network byte order
Print xid in host byte order so that it is easier to match the value to data in network packet. --- gdhcp/client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdhcp/client.c b/gdhcp/client.c index a031501..db21e01 100644 --- a/gdhcp/client.c +++ b/gdhcp/client.c @@ -2329,7 +2329,7 @@ static gboolean listener_event(GIOChannel *channel, GIOCondition condition, return TRUE; debug(dhcp_client, received DHCP packet xid 0x%04x - (current state %d), xid, dhcp_client-state); + (current state %d), ntohl(xid), dhcp_client-state); switch (dhcp_client-state) { case INIT_SELECTING: -- 1.8.3.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 0/4] Misc DHCPv4 patches
Hi, there has been some reports that sometimes ConnMan is not able to get IP address from the server. This was eventually traced to broadcast flag that is transmitted in DHCP packets. Some buggy AP discard the DHCP packet if broadcast flag is present and some discard it if it is missing. This is a nasty problem that patch 4 tries to overcome. As a workaround we send two packets in initial handshake where the other has broadcast flag and the other does not. If the server is discarding the other packet, we notice that and then can find out what kind of packets the server is able to accept. While doing the above workaround, I noticed some other issue in gdhcp code. This are fixed in patches 1-3. Cheers, Jukka Jukka Rissanen (4): gdhcp: Transaction id was not set when receiving L2 packet gdhcp: Transaction id is in network byte order gdhcp: Fix memory leak when clearing option hash gdhcp: Workaround for buggy AP that handle broadcast flag incorrectly gdhcp/client.c | 76 +++--- gdhcp/common.c | 8 --- gdhcp/common.h | 3 ++- gdhcp/server.c | 4 ++-- 4 files changed, 77 insertions(+), 14 deletions(-) -- 1.8.3.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 3/4] gdhcp: Fix memory leak when clearing option hash
The option list was not freed after individual options were removed. This is the valgrind report of the issue ==16972== 24 bytes in 1 blocks are definitely lost in loss record 81 of 262 ==16972==at 0x4C2745D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16972==by 0x4E7FE6E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E95FAD: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E76A63: g_list_append (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x41297A: listener_event (client.c:1867) ==16972==by 0x4E7A2A5: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E7A627: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E7AA39: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x40FDF6: main (main.c:689) ==16972== ==16972== 192 (144 direct, 48 indirect) bytes in 6 blocks are definitely lost in loss record 227 of 262 ==16972==at 0x4C2745D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16972==by 0x4E7FE6E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E95FAD: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E76A63: g_list_append (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4126F5: listener_event (client.c:1877) ==16972==by 0x4E7A2A5: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E7A627: ??? (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x4E7AA39: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3800.2) ==16972==by 0x40FDF6: main (main.c:689) --- gdhcp/client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gdhcp/client.c b/gdhcp/client.c index db21e01..16fe080 100644 --- a/gdhcp/client.c +++ b/gdhcp/client.c @@ -1130,6 +1130,7 @@ static void remove_option_value(gpointer data) GList *option_value = data; g_list_foreach(option_value, remove_value, NULL); + g_list_free(option_value); } GDHCPClient *g_dhcp_client_new(GDHCPType type, -- 1.8.3.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH v2 0/3] Peer service registration fixes
This set aims to solve observed issues during peer service register and unregister scenarios. Eduardo Abinader (3): gsupplicant: Check interface on interface_has_p2p wifi: Check peer_service user_data before invoking cb peer: Unset peer service driver upon peer unregister gsupplicant/supplicant.c | 3 +++ plugins/wifi.c | 6 +++--- src/peer.c | 2 ++ 3 files changed, 8 insertions(+), 3 deletions(-) -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 1/3] gsupplicant: Check interface on interface_has_p2p
Solves a segmentation fault occuring on peer service registration, when a wifi plugin did not have an associated interface while looping in peer service registration. --- gsupplicant/supplicant.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gsupplicant/supplicant.c b/gsupplicant/supplicant.c index fd16caf..b5e3930 100644 --- a/gsupplicant/supplicant.c +++ b/gsupplicant/supplicant.c @@ -3181,6 +3181,9 @@ int g_supplicant_interface_set_country(GSupplicantInterface *interface, bool g_supplicant_interface_has_p2p(GSupplicantInterface *interface) { + if (!interface) + return false; + return interface-p2p_support; } -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 3/3] peer: Unset peer service driver upon peer unregister
When peer driver is unregistered, peer service driver should be unregistered as well, in order to ensure proper queueing of peer service registration for later conclusion, when a p2p enabled device is connected. --- src/peer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/peer.c b/src/peer.c index 5f2a699..d339dd3 100644 --- a/src/peer.c +++ b/src/peer.c @@ -1012,6 +1012,8 @@ void connman_peer_driver_unregister(struct connman_peer_driver *driver) return; peer_driver = NULL; + + __connman_peer_service_set_driver(NULL); } void __connman_peer_list_struct(DBusMessageIter *array) -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH 2/3] wifi: Check peer_service user_data before invoking cb
A check was added, due to the occurrence of peer service registration without callback. Such scenarios takes place, when a previous peer service was registered with a proper callback, on a distinct interface. --- plugins/wifi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/wifi.c b/plugins/wifi.c index f16c3fe..45d88b8 100644 --- a/plugins/wifi.c +++ b/plugins/wifi.c @@ -376,10 +376,10 @@ static void register_peer_service_cb(int result, wifi-servicing++; } - if (reg_data-callback) + if (reg_data reg_data-callback) { reg_data-callback(result, reg_data-user_data); - - g_free(reg_data); + g_free(reg_data); + } } static GSupplicantP2PServiceParams *fill_in_peer_service_params( -- 1.9.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
[PATCH] Fix fallback nameservers harder
From: Andreas Hartmetz ahartm...@gmail.com - Make sure to insert them with the enabled flag set. No need to touch the flag afterwards. When they are not supposed to be used, they are removed entirely anyway. - Insert the fallbacks also when taking some service offline. As I understand it, the fallbacks are for all cases where there is no other nameserver, including with everything ConnMan knows about disconnected. It may or may not have originally been meant that way, in any case it's what we need in our current project. --- Hi, sending this to ml on behalf of Andreas so that the patch is not lost. Cheers, Jukka src/dnsproxy.c | 5 + src/service.c | 1 + 2 files changed, 6 insertions(+) diff --git a/src/dnsproxy.c b/src/dnsproxy.c index 741cd45..ba1bd1e 100644 --- a/src/dnsproxy.c +++ b/src/dnsproxy.c @@ -2603,6 +2603,7 @@ static struct server_data *create_server(int index, if (protocol == IPPROTO_UDP) { if (__connman_service_index_is_default(data-index) || + data-index 0 || __connman_service_index_is_split_routing( data-index)) { data-enabled = true; @@ -2788,6 +2789,10 @@ static void dnsproxy_offline_mode(bool enabled) for (list = server_list; list; list = list-next) { struct server_data *data = list-data; + /* fallback nameservers (index 0) are always enabled if present */ + if (data-index 0) + continue; + if (!enabled) { DBG(Enabling DNS server %s, data-server); data-enabled = true; diff --git a/src/service.c b/src/service.c index e284e92..1e2218b 100644 --- a/src/service.c +++ b/src/service.c @@ -1048,6 +1048,7 @@ static void update_nameservers(struct connman_service *service) case CONNMAN_SERVICE_STATE_FAILURE: case CONNMAN_SERVICE_STATE_DISCONNECT: connman_resolver_remove_all(index); + connman_resolver_flush(); return; case CONNMAN_SERVICE_STATE_READY: case CONNMAN_SERVICE_STATE_ONLINE: -- 1.8.3.1 ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH v2 0/3] Peer service registration fixes
ACK on these ones Thanks ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Issue doing Connect on a failed service
Hi, Currently there is an issue in connman when Connect is issued for a failed service. 1. In connect service the incoming DBusMessage is assigned to service-pending. 2. __connman_service_connect is called which will call __connman_service_clear_error. state_ipv4 and state_ipv6 is set to unknown. In the following call to service_inidicate_state, its found that we transition to idle from failure, resulting in a call to __connman_service_disconnect. 3. In disconnect reply_pending is called which will take the DBusMessage and reply with an error. This is new behaviour since the commit: 127d216001076d2b73a352a019f00d49bae30835 I'm not 100% sure what issue the commit fixed. I guess we could go back and let service_indicate_state only call disconnect in idle if previous state wasnt disconnected OR failure. But that might roll back the fix of the commit. We could also avoid to attach the message (step 1.) but a lot of other error checks would than fail... Happy for any input :-) --Richard ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman
Re: [PATCH 4/4] gdhcp: Workaround for buggy AP that handle broadcast flag incorrectly
Hi Jukka, Some routers/AP handle the DHCP broadcast flag incorrectly. This means that some AP discard the DHCP packet if broadcast flag is present and some discard it if broadcast flag is missing. The workaround is to send two DISCOVER packets in INIT state and two REQUEST packets in REBOOTING state. The first packet does not set the broadcast flag and the latter one sets it. When response is received, we check what kind of destination address (unicast or broadcast) the server used and start to send packets to it having either broadcast flag set or not. --- gdhcp/client.c | 72 -- gdhcp/common.c | 8 --- gdhcp/common.h | 3 ++- gdhcp/server.c | 4 ++-- 4 files changed, 74 insertions(+), 13 deletions(-) diff --git a/gdhcp/client.c b/gdhcp/client.c index 16fe080..238a301 100644 --- a/gdhcp/client.c +++ b/gdhcp/client.c @@ -155,6 +155,7 @@ struct _GDHCPClient { uint32_t expire; bool retransmit; struct timeval start_time; + bool request_bcast; }; static inline void debug(GDHCPClient *client, const char *format, ...) @@ -436,6 +437,7 @@ static uint16_t dhcp_attempt_secs(GDHCPClient *dhcp_client) static int send_discover(GDHCPClient *dhcp_client, uint32_t requested) { struct dhcp_packet packet; + int ret; debug(dhcp_client, sending DHCP discover request); @@ -455,15 +457,30 @@ static int send_discover(GDHCPClient *dhcp_client, uint32_t requested) add_send_options(dhcp_client, packet); + /* + * We send discover packet twice, first without broadcast flag and + * then with broadcast flag. Reason is various buggy routers/ap that + * either eat the other or vice versa. In the receiving side we then + * find out what kind of packet the server can send. + */ + ret = dhcp_send_raw_packet(packet, INADDR_ANY, CLIENT_PORT, + INADDR_BROADCAST, SERVER_PORT, + MAC_BCAST_ADDR, dhcp_client-ifindex, false); + if (ret 0) + return ret; + return dhcp_send_raw_packet(packet, INADDR_ANY, CLIENT_PORT, - INADDR_BROADCAST, SERVER_PORT, - MAC_BCAST_ADDR, dhcp_client-ifindex); + INADDR_BROADCAST, SERVER_PORT, + MAC_BCAST_ADDR, dhcp_client-ifindex, true); } I actually do not like the idea to send both at the same time. Has it been measured on what impact that might has on DHCP lease time. I rather have it send one first (maybe starting out with unicast) is safer here and have a timeout. If no response comes, we try the other type. Once we know which ones work and which ones doesn't. We store that internally for that service. And next we start with that type first. However same applies, if the type we start with timeouts, then we try the other one. That seems like a smarter logic that allows us to adapt to different network cases while only eating the penalty once. Regards Marcel ___ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman