@@ -1033,9 +1033,18 @@ static int wispr_portal_detect(struct connman_wispr_portal_context *wp_context)
* connman_proxy_lookup, since the former will always result in
* a WISPr request "falling down a hole" that will only ever
* result in a failure completion.
+ *
+ * Whether following the connman_proxy_lookup / proxy_callback
+ * path or the g_idle_add / no_proxy_callback path, both funnel to
+ * proxy_callback. Retain a reference to the WISPr/portal context
+ * here and release it there (that is, in proxy_callback) such
+ * that the context is retained while the lookup or the idle
+ * timeout are in flight.
*/
if (proxy_method != CONNMAN_SERVICE_PROXY_METHOD_DIRECT &&
proxy_method != CONNMAN_SERVICE_PROXY_METHOD_UNKNOWN) {
+ wispr_portal_context_ref(wp_context);
+
wp_context->token = connman_proxy_lookup(interface,
wp_context->status_url,
wp_context->service,
@@ -1046,6 +1055,8 @@ static int wispr_portal_detect(struct connman_wispr_portal_context *wp_context)
wispr_portal_context_unref(wp_context);
}
} else if (wp_context->timeout == 0) {
+ wispr_portal_context_ref(wp_context);
+
wp_context->timeout = g_idle_add(no_proxy_callback, wp_context);
}
This balances previously-unbalanced WISPr/portal context reference counting associated with proxy handling that can lead to a heap-use- after-free fault. Prior to this change, 'proxy_callback' contained an unbalanced release on the reference to a WISPr/portal context that could lead to its premature destructio and a subsequent heap-use-after-free fault in 'free_wispr_routes'. This situation could occur, due to this imbalance, when a new WISPr/portal request for given service and IP configuration combination displaced a prior, but unclosed and pending within 'g_web_request_get' request, WISPr/portal context leading to its premature deletion. Later, when the prior 'g_web_request_get' completed, either successfully or unsuccessfully, this resulted in a different, yet balanced release on the reference at the end of the 'wispr_portal_web_result' callback to 'g_web_request_get'. This also resulted in its (now redundant) destruction. The heap-use-after-free actually occurred BEFORE the reference release, in 'free_wispr_routes' since in 'wispr_portal_web_result', 'free_wispr_routes' is invoked BEFORE the reference is released. This imbalance is addressed by balancing the reference release in 'proxy_callback' with a reference retain prior to either invoking 'connman_proxy_lookup' with the 'proxy_callback' callback or invoking 'g_idle_add' with the 'no_proxy_callback', which ultimately calls 'proxy_callback'. The address sanitizer report associated with the potential heap-use- after-free is as follows: ================================================================= ==2188==ERROR: AddressSanitizer: heap-use-after-free on address 0x72b0f4b4 at pc 0x000f36b5 bp 0x7eac98c8 sp 0x7eac98cc READ of size 4 at 0x72b0f4b4 thread T0 #0 0xf36b2 in free_wispr_routes src/wispr.c:140 #1 0xf5fde in wispr_portal_web_result src/wispr.c:871 #2 0x3285e in call_result_func gweb/gweb.c:465 #3 0x3285e in received_data gweb/gweb.c:920 0x72b0f4b4 is located 100 bytes inside of 108-byte region [0x72b0f450,0x72b0f4bc) freed by thread T0 here: #0 0x76abf264 in __interceptor_free /data/jenkins/workspace/GNU-toolchain/arm-12/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0xf3916 in free_connman_wispr_portal_context src/wispr.c:211 #2 0xf4706 in wispr_portal_context_unref_debug src/wispr.c:239 #3 0xf6408 in __connman_wispr_start src/wispr.c:1101 #4 0x9830a in __connman_service_wispr_start src/service.c:2421 #5 0x990d2 in start_wispr_if_connected src/service.c:2375 #6 0x9ae2c in default_changed src/service.c:2637 #7 0x9afb6 in __connman_service_indicate_default src/service.c:7660 #8 0x856de in set_default_gateway src/connection.c:509 #9 0x86e3a in __connman_connection_update_gateway src/connection.c:1053 #10 0x9b0aa in service_update_preferred_order src/service.c:7320 #11 0x9b2d2 in service_indicate_state src/service.c:7400 #12 0x9baaa in __connman_service_ipconfig_indicate_state src/service.c:7848 #13 0x9ed28 in complete_online_check src/service.c:2219 #14 0xf44fc in portal_manage_success_status src/wispr.c:516 #15 0xf60ac in wispr_portal_web_result src/wispr.c:820 #16 0x3285e in call_result_func gweb/gweb.c:465 #17 0x3285e in received_data gweb/gweb.c:920 previously allocated by thread T0 here: #0 0x76abfa6c in __interceptor_calloc /data/jenkins/workspace/GNU-toolchain/arm-12/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:77 SUMMARY: AddressSanitizer: heap-use-after-free src/wispr.c:140 in free_wispr_routes --- src/wispr.c | 11 +++++++++++ 1 file changed, 11 insertions(+)