>-----Original Message----- >From: [email protected] [mailto:[email protected]] On >Behalf Of Xu, Martin >Sent: 2009年6月27日 22:49 >To: [email protected] >Cc: Liu, Bing Wei; Shureih, Tariq >Subject: RE: patch to fix connect hidden Guest issue. bug 3445 > >>From: [email protected] [mailto:[email protected]] On >>Behalf Of Marcel Holtmann >>Sent: 2009年6月26日 18:47 >>To: [email protected] >>Cc: Liu, Bing Wei; Shureih, Tariq >>Subject: Re: patch to fix connect hidden Guest issue. bug 3445 >> >>Hi Martin, >> >>> I have worked out a patch to fix bug 3445. please review it. >>> >>> The root cause of the bug is: >>> 1. "group" is not set to the "network" created by ConnMan to associate a >>> hidden >>AP, so the corresponding service will not be created, and showed. >>> 2. However, you can find that the original hidden services visible. It is >>because that the associating with the hidden AP may force >>WPA_SUPPLICANT/driver >>active scan(probe/request) with the hidden AP and get the hidden name. >>Consequently, the original hidden services will be showed. But the service is >>not be linked to the connected network created by "Join" method. That is the >>reason why you find that the services state is not correct. >>> >>> 3. Currently, when supplicant find the hidden AP network visible, it will >>update the service using the AP/network. Because device_identifity and group >>issued to identify a service, and same SSID, encrypt and manage mode >>compromise >>group so if several APs in the same group. When suppplicant get these APs, it >>will update the same service using these networks one by one. Consequently, >the >>network linking with the last AP will be used to link with service. So the >service >>state is not right and all the operations to the service all have issues. >>> >>> The patch resolves the issues through below method: >>> 1. set the group to the network created by ConnMan, so the corresponding >service >>has chance to be created. >>> >>> Please review the patch >>> >>> diff --git a/src/device.c b/src/device.c >>> index cd9105d..0c97a9c 100644 >>> --- a/src/device.c >>> +++ b/src/device.c >>> @@ -526,6 +526,22 @@ static DBusMessage *set_property(DBusConnection *conn, >>> return g_dbus_create_reply(msg, DBUS_TYPE_INVALID); >>> } >>> >>> +static char *build_group(const unsigned char *ssid, >>> + unsigned int ssid_len, >>> + const char *mode, >>> + const char *security) >>> +{ >>> + int i; >>> + GString *str = g_string_sized_new((ssid_len * 2) + 24); >>> + if (ssid_len > 0 && ssid[0] != '\0') >>> + for (i = 0; i < ssid_len; i++) >>> + g_string_append_printf(str, "%02x", ssid[i]); >>> + >>> + g_string_append_printf(str, "_%s_%s", mode, security); >>> + >>> + return g_string_free(str, FALSE); >>> +} >>> + >>> static DBusMessage *join_network(DBusConnection *conn, >>> DBusMessage *msg, void *data) >>> { >>> @@ -534,7 +550,9 @@ static DBusMessage *join_network(DBusConnection *conn, >>> enum connman_network_type type; >>> DBusMessageIter iter, array; >>> int err, index; >>> - >>> + unsigned int ssid_size; >>> + const char *group, *mode, *security; >>> + const void *ssid; >>> DBG("conn %p", conn); >>> >>> if (__connman_security_check_privilege(msg, >>> @@ -583,6 +601,13 @@ static DBusMessage *join_network(DBusConnection *conn, >>> dbus_message_iter_next(&array); >>> } >>> >>> + ssid = connman_network_get_blob(network, "WiFi.SSID", &ssid_size); >>> + security = connman_network_get_string(network, "WiFi.Security"); >>> + mode = connman_network_get_string(network, "WiFi.Mode"); >>> + group = build_group(ssid, ssid_size, mode, security); >>> + DBG("group:%s", group); >>> + connman_network_set_group(network, group); >>> + >>> index = connman_device_get_index(device); >>> connman_network_set_index(network, index); >> >>I re-arranged this a little bit and applied it. Looks good to me. >> >>> diff --git a/src/service.c b/src/service.c >>> index aa89d5d..c4c4f30 100644 >>> --- a/src/service.c >>> +++ b/src/service.c >>> @@ -616,6 +616,14 @@ void __connman_service_put(struct connman_service >>*service) >>> { >>> DBG("service %p", service); >>> >>> + if (service->state == CONNMAN_SERVICE_STATE_CARRIER || >>> + service->state == CONNMAN_SERVICE_STATE_ASSOCIATION || >>> + service->state == CONNMAN_SERVICE_STATE_READY || >>> + service->state == CONNMAN_SERVICE_STATE_CONFIGURATION) { >>> + DBG("service is active and can not be put!"); >>> + return; >>> + } >>> + >>> if (g_atomic_int_dec_and_test(&service->refcount) == TRUE) { >>> GSequenceIter *iter; >> >>This is wrong for sure. What are you trying to fix. >The fix is critical. :-) >I think the issue it can be reproduce on office "Guest" environment, think of >several APs using same hidden ssid, encrypt. When associated, all these hidden >APs are not hidden anymore and each AP/network will update the services. This >is the root-cause of the bugs. I have done a carefully investigation on it to >find the issue. :-) > > >I re-past my analysis about the rootcause, it has described detailed. > >3. Currently, when supplicant find the hidden AP network visible, it will >update the service using the AP/network. Because device_identifity and group >issued to identify a service, and same SSID, encrypt and manage mode compromise >group so if several APs in the same group. When suppplicant get these APs, it >will update the same service using these networks one by one. Consequently, the >network linking with the last AP will be used to link with service. So the >service >state is not right and all the operations to the service all have issues. > >2. before updating service with the new network, check whether the original >network linked with the service are active. If it active, the update will not >go no. That makes sure the service always has the right network.
Marcel: I just verified the latest git, if the above patch can not be merged, the hidden-Guest service can not work properly, the behavior is the disconnect of the service can not work. The root cause is just like what I mentioned above. :-) _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
