Address -Werror=sign-compare Compilation Errors #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This addresses -Werror=sign-compare compilation errors surfaced by both GCC and clang/LLVM when --enable-maintainer-mode is asserted.
gerickson
deleted the
user/gerickson/github-issue-11-address-sign-compare-errors
branch
gerickson
added a commit
that referenced
this pull request
Nov 10, 2023
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
gerickson
added a commit
that referenced
this pull request
Nov 10, 2023
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
gerickson
added a commit
that referenced
this pull request
Nov 15, 2023
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This addresses #11, by using casts to eliminate
-Werror=sign-comparecompilation errors surfaced by both GCC and clang/LLVM when--enable-maintainer-modeis asserted.