Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix crashes when DeviceCommissioner is shut down. #22245

Conversation

bzbarsky-apple
Copy link
Contributor

There are two fixes here:

  1. Ensure that DeviceCommissioner::Shutdown properly cleans up
    mDeviceBeingCommissioned so we don't end up with it being a dangling pointer.

  2. Ensure that DeviceCommissioner::CommissioningStageComplete doesn't try to
    derefence a null mDeviceBeingCommissioned (which indicates something has
    already stopped the commissioning process).

Fixes #22243

Problem

See #22243 for crash analysis.

Change overview

See above.

Testing

CI should generally test this on happy paths. Trying to figure out how to write a test that would exercise the "shut down in the middle of commissioning" codepath.

There are two fixes here:

1) Ensure that DeviceCommissioner::Shutdown properly cleans up
   mDeviceBeingCommissioned so we don't end up with it being a dangling pointer.

2) Ensure that DeviceCommissioner::CommissioningStageComplete doesn't try to
   derefence a null mDeviceBeingCommissioned (which indicates something has
   already stopped the commissioning process).

Fixes project-chip#22243
@github-actions
Copy link

github-actions bot commented Aug 29, 2022

PR #22245: Size comparison from 6329339 to f6e0ff4

Increases (6 builds for cc13x2_26x2, linux, psoc6)
platform target config section 6329339 f6e0ff4 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read/write) 167680 167688 8 0.0
linux chip-tool debug (read only) 10902865 10902993 128 0.0
.text 8818484 8818612 128 0.0
chip-tool-ipv6only arm64 (read only) 10284372 10284484 112 0.0
.text 8137876 8137988 112 0.0
tv-app debug (read only) 3188281 3188409 128 0.0
.text 2738466 2738594 128 0.0
psoc6 all-clusters cy8ckit_062s2_43012 .debug_info 26696028 26696029 1 0.0
all-clusters-minimal cy8ckit_062s2_43012 .debug_info 26432723 26432724 1 0.0
Decreases (5 builds for cc13x2_26x2, esp32, psoc6, telink)
platform target config section 6329339 f6e0ff4 change % change
cc13x2_26x2 lock-ftd LP_CC2652R7 (read only) 674023 674015 -8 -0.0
.text 596872 596864 -8 -0.0
esp32 all-clusters-app m5stack (read/write) 490780 490772 -8 -0.0
.flash.rodata 247352 247344 -8 -0.0
psoc6 lock cy8ckit_062s2_43012 .debug_info 22275869 22275868 -1 -0.0
telink light-switch-app tlsr9518adk80d text 571204 571202 -2 -0.0
lighting-app tlsr9518adk80d text 589316 589314 -2 -0.0
Full report (43 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, telink)
platform target config section 6329339 f6e0ff4 change % change
bl602 lighting-app bl602 (read/write) 1385062 1385062 0 0.0
.bss 120274 120274 0 0.0
.data 4488 4488 0 0.0
.text 1051464 1051464 0 0.0
bl602+rpc (read/write) 1430958 1430958 0 0.0
.bss 127706 127706 0 0.0
.data 4600 4600 0 0.0
.text 1083480 1083480 0 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 672979 672979 0 0.0
(read/write) 178412 178412 0 0.0
.bss 74284 74284 0 0.0
.data 3372 3372 0 0.0
.rodata 88835 88835 0 0.0
.text 583828 583828 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 637683 637683 0 0.0
(read/write) 157844 157844 0 0.0
.bss 73556 73556 0 0.0
.data 3372 3372 0 0.0
.rodata 77979 77979 0 0.0
.text 559380 559380 0 0.0
lock-ftd LP_CC2652R7 (read only) 674023 674015 -8 -0.0
(read/write) 167680 167688 8 0.0
.bss 71484 71484 0 0.0
.data 3296 3296 0 0.0
.rodata 76671 76671 0 0.0
.text 596872 596864 -8 -0.0
lock-mtd LP_CC2652R7 (read only) 656955 656955 0 0.0
(read/write) 180436 180436 0 0.0
.bss 67172 67172 0 0.0
.data 3296 3296 0 0.0
.rodata 101883 101883 0 0.0
.text 554592 554592 0 0.0
pump-app LP_CC2652R7 (read only) 684739 684739 0 0.0
(read/write) 157668 157668 0 0.0
.bss 71420 71420 0 0.0
.data 3296 3296 0 0.0
.rodata 89947 89947 0 0.0
.text 594308 594308 0 0.0
pump-controller-app LP_CC2652R7 (read only) 669239 669239 0 0.0
(read/write) 173280 173280 0 0.0
.bss 71532 71532 0 0.0
.data 3292 3292 0 0.0
.rodata 85503 85503 0 0.0
.text 583256 583256 0 0.0
shell LP_CC2652R7 (read only) 665686 665686 0 0.0
(read/write) 181224 181224 0 0.0
.bss 76604 76604 0 0.0
.data 3376 3376 0 0.0
.rodata 85782 85782 0 0.0
.text 579588 579588 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 586690 586690 0 0.0
.app_xip_area 463348 463348 0 0.0
.bss 65776 65776 0 0.0
.data 744 744 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 592442 592442 0 0.0
.app_xip_area 464316 464316 0 0.0
.bss 70560 70560 0 0.0
.data 748 748 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 599434 599434 0 0.0
.app_xip_area 476812 476812 0 0.0
.bss 65088 65088 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1107504 1107504 0 0.0
.bss 136332 136332 0 0.0
.data 2072 2072 0 0.0
.text 969080 969080 0 0.0
BRD4161A+rpc (read/write) 971556 971556 0 0.0
.bss 150844 150844 0 0.0
.data 2252 2252 0 0.0
.text 818440 818440 0 0.0
BRD4161A+rs911x (read/write) 1001224 1001224 0 0.0
.bss 169088 169088 0 0.0
.data 2064 2064 0 0.0
.text 830052 830052 0 0.0
lock-app BRD4161A+wf200 (read/write) 1149716 1149716 0 0.0
.bss 152168 152168 0 0.0
.data 2072 2072 0 0.0
.text 995456 995456 0 0.0
window-app BRD4161A (read/write) 1098744 1098744 0 0.0
.bss 137772 137772 0 0.0
.data 2096 2096 0 0.0
.text 958856 958856 0 0.0
esp32 all-clusters-app c3devkit (read only) 1033498 1033498 0 0.0
(read/write) 1493486 1493486 0 0.0
.dram0.bss 71088 71088 0 0.0
.dram0.data 13696 13696 0 0.0
.flash.rodata 218032 218032 0 0.0
.flash.text 1033498 1033498 0 0.0
.iram0.text 65160 65160 0 0.0
m5stack (read only) 1085827 1085827 0 0.0
(read/write) 490780 490772 -8 -0.0
.dram0.bss 76608 76608 0 0.0
.dram0.data 34152 34152 0 0.0
.flash.rodata 247352 247344 -8 -0.0
.flash.text 1080443 1080443 0 0.0
.iram0.text 123939 123939 0 0.0
k32w light k32w0+release (read/write) 647292 647292 0 0.0
.bss 70424 70424 0 0.0
.data 2068 2068 0 0.0
.text 572072 572072 0 0.0
lock k32w0+release (read/write) 704288 704288 0 0.0
.bss 70864 70864 0 0.0
.data 2076 2076 0 0.0
.text 628620 628620 0 0.0
linux all-clusters-app debug (read only) 3043401 3043401 0 0.0
(read/write) 156032 156032 0 0.0
.bss 61792 61792 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 85768 85768 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1176 1176 0 0.0
.rodata 275307 275307 0 0.0
.text 2588658 2588658 0 0.0
all-clusters-minimal-app debug (read only) 2879185 2879185 0 0.0
(read/write) 147632 147632 0 0.0
.bss 61024 61024 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 78264 78264 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1160 1160 0 0.0
.rodata 275467 275467 0 0.0
.text 2427058 2427058 0 0.0
bridge-app debug+rpc (read only) 2377449 2377449 0 0.0
(read/write) 127752 127752 0 0.0
.bss 50656 50656 0 0.0
.data 3600 3600 0 0.0
.data.rel.ro 67640 67640 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 832 832 0 0.0
.rodata 204168 204168 0 0.0
.text 2010370 2010370 0 0.0
chip-tool debug (read only) 10902865 10902993 128 0.0
(read/write) 657384 657384 0 0.0
.bss 25240 25240 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 622352 622352 0 0.0
.dynamic 608 608 0 0.0
.got 5096 5096 0 0.0
.init 27 27 0 0.0
.init_array 776 776 0 0.0
.rodata 563541 563541 0 0.0
.text 8818484 8818612 128 0.0
chip-tool-ipv6only arm64 (read only) 10284372 10284484 112 0.0
(read/write) 705233 705233 0 0.0
.bss 33297 33297 0 0.0
.data 3280 3280 0 0.0
.data.rel.ro 649832 649832 0 0.0
.dynamic 560 560 0 0.0
.got 13848 13848 0 0.0
.init 24 24 0 0.0
.init_array 200 200 0 0.0
.rodata 494036 494036 0 0.0
.text 8137876 8137988 112 0.0
lighting-app debug+rpc (read only) 2602377 2602377 0 0.0
(read/write) 130536 130536 0 0.0
.bss 49792 49792 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72680 72680 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 928 928 0 0.0
.rodata 221008 221008 0 0.0
.text 2210178 2210178 0 0.0
lock-app debug (read only) 2585361 2585361 0 0.0
(read/write) 125712 125712 0 0.0
.bss 48288 48288 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69688 69688 0 0.0
.dynamic 608 608 0 0.0
.got 4464 4464 0 0.0
.init 27 27 0 0.0
.init_array 904 904 0 0.0
.rodata 238000 238000 0 0.0
.text 2180418 2180418 0 0.0
ota-provider-app debug (read only) 2362601 2362601 0 0.0
(read/write) 119144 119144 0 0.0
.bss 47808 47808 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63512 63512 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 768 768 0 0.0
.rodata 209976 209976 0 0.0
.text 1988866 1988866 0 0.0
ota-requestor-app debug (read only) 2527929 2527929 0 0.0
(read/write) 127552 127552 0 0.0
.bss 50368 50368 0 0.0
.data 2304 2304 0 0.0
.data.rel.ro 68920 68920 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 856 856 0 0.0
.rodata 216704 216704 0 0.0
.text 2138322 2138322 0 0.0
shell debug (read only) 2611689 2611689 0 0.0
(read/write) 142184 142184 0 0.0
.bss 57704 57704 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 77376 77376 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 1048 1048 0 0.0
.rodata 235410 235410 0 0.0
.text 2217570 2217570 0 0.0
thermostat-no-ble arm64 (read only) 2361556 2361556 0 0.0
(read/write) 141857 141857 0 0.0
.bss 55233 55233 0 0.0
.data 1680 1680 0 0.0
.data.rel.ro 76112 76112 0 0.0
.dynamic 560 560 0 0.0
.got 5056 5056 0 0.0
.init 24 24 0 0.0
.init_array 416 416 0 0.0
.rodata 141276 141276 0 0.0
.text 1982240 1982240 0 0.0
tv-app debug (read only) 3188281 3188409 128 0.0
(read/write) 258040 258040 0 0.0
.bss 167352 167352 0 0.0
.data 4752 4752 0 0.0
.data.rel.ro 79368 79368 0 0.0
.dynamic 608 608 0 0.0
.got 4856 4856 0 0.0
.init 27 27 0 0.0
.init_array 1080 1080 0 0.0
.rodata 259784 259784 0 0.0
.text 2738466 2738594 128 0.0
tv-casting-app debug (read only) 5508945 5508945 0 0.0
(read/write) 160536 160536 0 0.0
.bss 51352 51352 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 100304 100304 0 0.0
.dynamic 608 608 0 0.0
.got 4776 4776 0 0.0
.init 27 27 0 0.0
.init_array 1048 1048 0 0.0
.rodata 344945 344945 0 0.0
.text 4892098 4892098 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2454872 2454872 0 0.0
.bss 215044 215044 0 0.0
.data 5872 5872 0 0.0
.text 1417516 1417516 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1180691 1180691 0 0.0
bss 143641 143641 0 0.0
rodata 143380 143380 0 0.0
text 814724 814724 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1159871 1159871 0 0.0
bss 142868 142868 0 0.0
rodata 134968 134968 0 0.0
text 803116 803116 0 0.0
psoc6 all-clusters cy8ckit_062s2_43012 (read only) 842216 842216 0 0.0
(read/write) 1741700 1741700 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 188464 188464 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1221325 1221325 0 0.0
.debug_aranges 111696 111696 0 0.0
.debug_frame 372812 372812 0 0.0
.debug_info 26696028 26696029 1 0.0
.debug_line 3655093 3655093 0 0.0
.debug_loc 3568177 3568177 0 0.0
.debug_ranges 337560 337560 0 0.0
.debug_str 3426632 3426632 0 0.0
.heap 842216 842216 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 570457 570457 0 0.0
.symtab 421424 421424 0 0.0
.text 1542184 1542184 0 0.0
.zero.table 8 8 0 0.0
text 0 0 0 0.0
all-clusters-minimal cy8ckit_062s2_43012 (read only) 842952 842952 0 0.0
(read/write) 1684868 1684868 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 187728 187728 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2664 2664 0 0.0
.debug_abbrev 1213164 1213164 0 0.0
.debug_aranges 111168 111168 0 0.0
.debug_frame 375892 375892 0 0.0
.debug_info 26432723 26432724 1 0.0
.debug_line 3675609 3675609 0 0.0
.debug_loc 3555814 3555814 0 0.0
.debug_ranges 336176 336176 0 0.0
.debug_str 3415637 3415637 0 0.0
.heap 842952 842952 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 534931 534931 0 0.0
.symtab 408016 408016 0 0.0
.text 1486088 1486088 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
light cy8ckit_062s2_43012 (read only) 851184 851184 0 0.0
(read/write) 1602172 1602172 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 179704 179704 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2456 2456 0 0.0
.debug_abbrev 1047983 1047983 0 0.0
.debug_aranges 103344 103344 0 0.0
.debug_frame 346160 346160 0 0.0
.debug_info 21896230 21896230 0 0.0
.debug_line 3246078 3246078 0 0.0
.debug_loc 3254196 3254196 0 0.0
.debug_ranges 301648 301648 0 0.0
.debug_str 3220857 3220857 0 0.0
.heap 851184 851184 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 468230 468230 0 0.0
.symtab 375104 375104 0 0.0
.text 1411624 1411624 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
lock cy8ckit_062s2_43012 (read only) 846152 846152 0 0.0
(read/write) 1639844 1639844 0 0.0
.ARM.attributes 46 46 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 184720 184720 0 0.0
.comment 204 204 0 0.0
.copy.table 24 24 0 0.0
.cy_m0p_image 6216 6216 0 0.0
.cy_sharedmem 8 8 0 0.0
.data 2472 2472 0 0.0
.debug_abbrev 1055418 1055418 0 0.0
.debug_aranges 104016 104016 0 0.0
.debug_frame 348988 348988 0 0.0
.debug_info 22275869 22275868 -1 -0.0
.debug_line 3254895 3254895 0 0.0
.debug_loc 3293990 3293990 0 0.0
.debug_ranges 304992 304992 0 0.0
.debug_str 3248278 3248278 0 0.0
.heap 846152 846152 0 0.0
.noinit 148 148 0 0.0
.ramVectors 736 736 0 0.0
.shstrtab 288 288 0 0.0
.stab 156 156 0 0.0
.stabstr 335 335 0 0.0
.stack_dummy 4096 4096 0 0.0
.strtab 474445 474445 0 0.0
.symtab 378288 378288 0 0.0
.text 1444264 1444264 0 0.0
.zero.table 0 0 0 0.0
8 8 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 808600 808600 0 0.0
bss 71344 71344 0 0.0
noinit 43488 43488 0 0.0
text 571204 571202 -2 -0.0
lighting-app tlsr9518adk80d (read/write) 830500 830500 0 0.0
bss 72200 72200 0 0.0
noinit 43488 43488 0 0.0
text 589316 589314 -2 -0.0

@bzbarsky-apple bzbarsky-apple merged commit 5f46524 into project-chip:master Aug 30, 2022
@bzbarsky-apple bzbarsky-apple deleted the fix-controller-shutdown-crash branch August 30, 2022 13:02
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
There are two fixes here:

1) Ensure that DeviceCommissioner::Shutdown properly cleans up
   mDeviceBeingCommissioned so we don't end up with it being a dangling pointer.

2) Ensure that DeviceCommissioner::CommissioningStageComplete doesn't try to
   derefence a null mDeviceBeingCommissioned (which indicates something has
   already stopped the commissioning process).

Fixes project-chip#22243
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash if CHIPDeviceController shuts down in the middle of commissioning
3 participants