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.

Sign up for GitHub

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

Jump to bottom

Fix crash if commissionee drops PASE session before AddNOC. #21121

Merged
merged 1 commit into from
Jul 25, 2022
Merged

Fix crash if commissionee drops PASE session before AddNOC. #21121

merged 1 commit into from
Jul 25, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

Several changes here:

  1. Fix ReleaseCommissioneeDevice so it does not leave mDeviceBeingCommissioned
    dangling.
  2. Ensure that we have null-checks for mDeviceBeingCommissioned before all
    uses, in case it got nulled out by StopPairing or the like while some async
    operation was in flight. This changes:
    • DeviceCommissioner::DisarmDone
    • DeviceCommissioner::OnDeviceAttestationInformationVerification
  3. Remove kSessionEstablishmentTimeout so we don't have a random hardcoded
    timeout partway through commissioning.

Fixes #21106
Fixes #14650

Problem

See #21106 (comment)

Change overview

See above.

Testing

Tested using the test steps from #21106 (comment) and verified that we no longer crash.

@bzbarsky-apple
Fix crash if commissionee drops PASE session before AddNOC.
d1ed23d 
Several changes here:

1. Fix ReleaseCommissioneeDevice so it does not leave mDeviceBeingCommissioned
   dangling.
2. Ensure that we have null-checks for mDeviceBeingCommissioned before all
   uses, in case it got nulled out by StopPairing or the like while some async
   operation was in flight.  This changes:
   * DeviceCommissioner::DisarmDone
   * DeviceCommissioner::OnDeviceAttestationInformationVerification
3. Remove kSessionEstablishmentTimeout so we don't have a random hardcoded
   timeout partway through commissioning.

Fixes project-chip#21106
Fixes project-chip#14650
@github-actions
Copy link

github-actions bot commented Jul 22, 2022 •
edited
Loading

PR #21121: Size comparison from f219351 to d1ed23d

Increases (5 builds for bl602, linux)
platform target config section f219351 d1ed23d change % change
bl602 lighting-app bl602 .text 1050928 1050932 4 0.0
bl602+rpc .text 1082584 1082588 4 0.0
linux chip-tool debug .rodata 522005 522101 96 0.0
chip-tool-ipv6only arm64 .rodata 457724 457812 88 0.0
tv-app debug .rodata 250600 250696 96 0.0
Decreases (4 builds for esp32, linux)
platform target config section f219351 d1ed23d change % change
esp32 all-clusters-app c3devkit (read only) 1021974 1021972 -2 -0.0
.flash.text 1021974 1021972 -2 -0.0
linux chip-tool debug (read only) 10349921 10349457 -464 -0.0
.text 8374324 8373764 -560 -0.0
chip-tool-ipv6only arm64 (read only) 9779356 9778916 -440 -0.0
(read/write) 679441 679425 -16 -0.0
.got 13552 13544 -8 -0.1
.text 7737236 7736740 -496 -0.0
tv-app debug (read only) 3116193 3115713 -480 -0.0
.text 2677170 2676594 -576 -0.0
Full report (42 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, nrfconnect, p6, telink)
platform target config section f219351 d1ed23d change % change
bl602 lighting-app bl602 (read/write) 1380802 1380802 0 0.0
.bss 117474 117474 0 0.0
.data 4480 4480 0 0.0
.text 1050928 1050932 4 0.0
bl602+rpc (read/write) 1426210 1426210 0 0.0
.bss 124922 124922 0 0.0
.data 4600 4600 0 0.0
.text 1082584 1082588 4 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668431 668431 0 0.0
(read/write) 182912 182912 0 0.0
.bss 74236 74236 0 0.0
.data 3356 3356 0 0.0
.rodata 88359 88359 0 0.0
.text 579756 579756 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634031 634031 0 0.0
(read/write) 157804 157804 0 0.0
.bss 73532 73532 0 0.0
.data 3356 3356 0 0.0
.rodata 77583 77583 0 0.0
.text 556124 556124 0 0.0
lock-ftd LP_CC2652R7 (read only) 671635 671635 0 0.0
(read/write) 169884 169884 0 0.0
.bss 71300 71300 0 0.0
.data 3280 3280 0 0.0
.rodata 76475 76475 0 0.0
.text 594680 594680 0 0.0
lock-mtd LP_CC2652R7 (read only) 653895 653895 0 0.0
(read/write) 183312 183312 0 0.0
.bss 66988 66988 0 0.0
.data 3280 3280 0 0.0
.rodata 101191 101191 0 0.0
.text 552224 552224 0 0.0
pump-app LP_CC2652R7 (read only) 681319 681319 0 0.0
(read/write) 161056 161056 0 0.0
.bss 71388 71388 0 0.0
.data 3280 3280 0 0.0
.rodata 89207 89207 0 0.0
.text 591628 591628 0 0.0
pump-controller-app LP_CC2652R7 (read only) 667095 667095 0 0.0
(read/write) 175400 175400 0 0.0
.bss 71508 71508 0 0.0
.data 3276 3276 0 0.0
.rodata 85071 85071 0 0.0
.text 581544 581544 0 0.0
shell LP_CC2652R7 (read only) 660898 660898 0 0.0
(read/write) 185948 185948 0 0.0
.bss 76540 76540 0 0.0
.data 3360 3360 0 0.0
.rodata 85130 85130 0 0.0
.text 575452 575452 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 582082 582082 0 0.0
.app_xip_area 460472 460472 0 0.0
.bss 64404 64404 0 0.0
.data 716 716 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 587998 587998 0 0.0
.app_xip_area 461660 461660 0 0.0
.bss 69132 69132 0 0.0
.data 720 720 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 586534 586534 0 0.0
.app_xip_area 465772 465772 0 0.0
.bss 63612 63612 0 0.0
.data 660 660 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1087720 1087720 0 0.0
.bss 133220 133220 0 0.0
.data 2048 2048 0 0.0
.text 952432 952432 0 0.0
BRD4161A+rpc (read/write) 1141996 1141996 0 0.0
.bss 149892 149892 0 0.0
.data 2260 2260 0 0.0
.text 989820 989820 0 0.0
BRD4161A+rs911x (read/write) 972940 972940 0 0.0
.bss 161664 161664 0 0.0
.data 2048 2048 0 0.0
.text 809208 809208 0 0.0
lock-app BRD4161A+wf200 (read/write) 1128464 1128464 0 0.0
.bss 144304 144304 0 0.0
.data 2056 2056 0 0.0
.text 982084 982084 0 0.0
window-app BRD4161A (read/write) 1081196 1081196 0 0.0
.bss 134692 134692 0 0.0
.data 2076 2076 0 0.0
.text 944408 944408 0 0.0
esp32 all-clusters-app c3devkit (read only) 1021974 1021972 -2 -0.0
(read/write) 1486466 1486466 0 0.0
.dram0.bss 70232 70232 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216192 216192 0 0.0
.flash.text 1021974 1021972 -2 -0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1075751 1075751 0 0.0
(read/write) 488472 488472 0 0.0
.dram0.bss 75752 75752 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246580 246580 0 0.0
.flash.text 1070367 1070367 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w061+release (read/write) 660816 660816 0 0.0
.bss 69668 69668 0 0.0
.data 1992 1992 0 0.0
.text 583356 583356 0 0.0
lock k32w061+release (read/write) 687868 687868 0 0.0
.bss 70140 70140 0 0.0
.data 2004 2004 0 0.0
.text 609924 609924 0 0.0
linux all-clusters-app debug (read only) 2981817 2981817 0 0.0
(read/write) 155344 155344 0 0.0
.bss 61792 61792 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 85224 85224 0 0.0
.dynamic 608 608 0 0.0
.got 4568 4568 0 0.0
.init 27 27 0 0.0
.init_array 1064 1064 0 0.0
.rodata 265899 265899 0 0.0
.text 2537778 2537778 0 0.0
all-clusters-minimal-app debug (read only) 2827713 2827713 0 0.0
(read/write) 147016 147016 0 0.0
.bss 60992 60992 0 0.0
.data 2064 2064 0 0.0
.data.rel.ro 77784 77784 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 1056 1056 0 0.0
.rodata 266763 266763 0 0.0
.text 2385490 2385490 0 0.0
bridge-app debug+rpc (read only) 2342417 2342417 0 0.0
(read/write) 126920 126920 0 0.0
.bss 50080 50080 0 0.0
.data 3824 3824 0 0.0
.data.rel.ro 67240 67240 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 199400 199400 0 0.0
.text 1980706 1980706 0 0.0
chip-tool debug (read only) 10349921 10349457 -464 -0.0
(read/write) 631912 631912 0 0.0
.bss 24760 24760 0 0.0
.data 3266 3266 0 0.0
.data.rel.ro 597480 597480 0 0.0
.dynamic 608 608 0 0.0
.got 5088 5088 0 0.0
.init 27 27 0 0.0
.init_array 648 648 0 0.0
.rodata 522005 522101 96 0.0
.text 8374324 8373764 -560 -0.0
chip-tool-ipv6only arm64 (read only) 9779356 9778916 -440 -0.0
(read/write) 679441 679425 -16 -0.0
.bss 32833 32833 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 624824 624824 0 0.0
.dynamic 560 560 0 0.0
.got 13552 13544 -8 -0.1
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 457724 457812 88 0.0
.text 7737236 7736740 -496 -0.0
lighting-app debug+rpc (read only) 2565569 2565569 0 0.0
(read/write) 129888 129888 0 0.0
.bss 49632 49632 0 0.0
.data 2096 2096 0 0.0
.data.rel.ro 72296 72296 0 0.0
.dynamic 608 608 0 0.0
.got 4392 4392 0 0.0
.init 27 27 0 0.0
.init_array 824 824 0 0.0
.rodata 215088 215088 0 0.0
.text 2180082 2180082 0 0.0
lock-app debug (read only) 2530561 2530561 0 0.0
(read/write) 124872 124872 0 0.0
.bss 48032 48032 0 0.0
.data 1712 1712 0 0.0
.data.rel.ro 69272 69272 0 0.0
.dynamic 608 608 0 0.0
.got 4424 4424 0 0.0
.init 27 27 0 0.0
.init_array 800 800 0 0.0
.rodata 230096 230096 0 0.0
.text 2134866 2134866 0 0.0
ota-provider-app debug (read only) 2334177 2334177 0 0.0
(read/write) 118672 118672 0 0.0
.bss 47680 47680 0 0.0
.data 1936 1936 0 0.0
.data.rel.ro 63256 63256 0 0.0
.dynamic 608 608 0 0.0
.got 4488 4488 0 0.0
.init 27 27 0 0.0
.init_array 680 680 0 0.0
.rodata 204920 204920 0 0.0
.text 1966050 1966050 0 0.0
ota-requestor-app debug (read only) 2452633 2452633 0 0.0
(read/write) 125576 125576 0 0.0
.bss 50016 50016 0 0.0
.data 2240 2240 0 0.0
.data.rel.ro 67480 67480 0 0.0
.dynamic 608 608 0 0.0
.got 4480 4480 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 208640 208640 0 0.0
.text 2072386 2072386 0 0.0
shell debug (read only) 2568425 2568425 0 0.0
(read/write) 141400 141400 0 0.0
.bss 57608 57608 0 0.0
.data 1248 1248 0 0.0
.data.rel.ro 76856 76856 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 229202 229202 0 0.0
.text 2181666 2181666 0 0.0
thermostat-no-ble arm64 (read only) 2342012 2342012 0 0.0
(read/write) 141265 141265 0 0.0
.bss 55233 55233 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75616 75616 0 0.0
.dynamic 560 560 0 0.0
.got 4984 4984 0 0.0
.init 24 24 0 0.0
.init_array 400 400 0 0.0
.rodata 139604 139604 0 0.0
.text 1965568 1965568 0 0.0
tv-app debug (read only) 3116193 3115713 -480 -0.0
(read/write) 257024 257024 0 0.0
.bss 167064 167064 0 0.0
.data 4736 4736 0 0.0
.data.rel.ro 78792 78792 0 0.0
.dynamic 608 608 0 0.0
.got 4848 4848 0 0.0
.init 27 27 0 0.0
.init_array 960 960 0 0.0
.rodata 250600 250696 96 0.0
.text 2677170 2676594 -576 -0.0
tv-casting-app debug (read only) 5369905 5369905 0 0.0
(read/write) 158328 158328 0 0.0
.bss 51256 51256 0 0.0
.data 2432 2432 0 0.0
.data.rel.ro 98352 98352 0 0.0
.dynamic 608 608 0 0.0
.got 4736 4736 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 334785 334785 0 0.0
.text 4768594 4768594 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1177131 1177131 0 0.0
bss 143068 143068 0 0.0
rodata 142596 142596 0 0.0
text 812608 812608 0 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1157203 1157203 0 0.0
bss 142304 142304 0 0.0
rodata 134132 134132 0 0.0
text 801924 801924 0 0.0
p6 all-clusters-app default (read only) 881632 881632 0 0.0
(read/write) 1686660 1686660 0 0.0
.bss 149064 149064 0 0.0
.data 2648 2648 0 0.0
.text 1526560 1526560 0 0.0
all-clusters-minimal-app default (read only) 882352 882352 0 0.0
(read/write) 1630772 1630772 0 0.0
.bss 148344 148344 0 0.0
.data 2648 2648 0 0.0
.text 1471392 1471392 0 0.0
light-app default (read only) 890656 890656 0 0.0
(read/write) 1551020 1551020 0 0.0
.bss 140248 140248 0 0.0
.data 2440 2440 0 0.0
.text 1399944 1399944 0 0.0
lock-app default (read only) 886184 886184 0 0.0
(read/write) 1588612 1588612 0 0.0
.bss 144704 144704 0 0.0
.data 2456 2456 0 0.0
.text 1433064 1433064 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799664 799664 0 0.0
bss 70744 70744 0 0.0
noinit 40416 40416 0 0.0
text 567228 567228 0 0.0
lighting-app tlsr9518adk80d (read/write) 819712 819712 0 0.0
bss 71588 71588 0 0.0
noinit 40416 40416 0 0.0
text 583772 583772 0 0.0

@woody-apple woody-apple merged commit be1d492 into project-chip:master Jul 25, 2022
github-actions bot pushed a commit that referenced this pull request Jul 25, 2022
@bzbarsky-apple @web-flow
Fix crash if commissionee drops PASE session before AddNOC. (#21121)
d630d07 
Several changes here:

1. Fix ReleaseCommissioneeDevice so it does not leave mDeviceBeingCommissioned
   dangling.
2. Ensure that we have null-checks for mDeviceBeingCommissioned before all
   uses, in case it got nulled out by StopPairing or the like while some async
   operation was in flight.  This changes:
   * DeviceCommissioner::DisarmDone
   * DeviceCommissioner::OnDeviceAttestationInformationVerification
3. Remove kSessionEstablishmentTimeout so we don't have a random hardcoded
   timeout partway through commissioning.

Fixes #21106
Fixes #14650
@bzbarsky-apple bzbarsky-apple deleted the fix-timeout-crash branch July 25, 2022 18:43
woody-apple added a commit that referenced this pull request Jul 25, 2022
@woody-apple @bzbarsky-apple
Fix crash if commissionee drops PASE session before AddNOC. (#21121) (#…
45fa932 
…21167)

Several changes here:

1. Fix ReleaseCommissioneeDevice so it does not leave mDeviceBeingCommissioned
   dangling.
2. Ensure that we have null-checks for mDeviceBeingCommissioned before all
   uses, in case it got nulled out by StopPairing or the like while some async
   operation was in flight.  This changes:
   * DeviceCommissioner::DisarmDone
   * DeviceCommissioner::OnDeviceAttestationInformationVerification
3. Remove kSessionEstablishmentTimeout so we don't have a random hardcoded
   timeout partway through commissioning.

Fixes #21106
Fixes #14650

Co-authored-by: Boris Zbarsky <[email protected]>
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
@bzbarsky-apple @isiu-apple
Fix crash if commissionee drops PASE session before AddNOC. (project-…
fcc22ab 
…chip#21121)

Several changes here:

1. Fix ReleaseCommissioneeDevice so it does not leave mDeviceBeingCommissioned
   dangling.
2. Ensure that we have null-checks for mDeviceBeingCommissioned before all
   uses, in case it got nulled out by StopPairing or the like while some async
   operation was in flight.  This changes:
   * DeviceCommissioner::DisarmDone
   * DeviceCommissioner::OnDeviceAttestationInformationVerification
3. Remove kSessionEstablishmentTimeout so we don't have a random hardcoded
   timeout partway through commissioning.

Fixes project-chip#21106
Fixes project-chip#14650
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woody-apple woody-apple woody-apple approved these changes

@msandstedt msandstedt msandstedt approved these changes

@andy31415 andy31415 Awaiting requested review from andy31415

@anush-apple anush-apple Awaiting requested review from anush-apple

@arkq arkq Awaiting requested review from arkq

@Byungjoo-Lee Byungjoo-Lee Awaiting requested review from Byungjoo-Lee

@carol-apple carol-apple Awaiting requested review from carol-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@chshu chshu Awaiting requested review from chshu

@chulspro chulspro Awaiting requested review from chulspro

@Damian-Nordic Damian-Nordic Awaiting requested review from Damian-Nordic

@dhrishi dhrishi Awaiting requested review from dhrishi

@electrocucaracha electrocucaracha Awaiting requested review from electrocucaracha

@emargolis emargolis Awaiting requested review from emargolis

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@franck-apple franck-apple Awaiting requested review from franck-apple

@gjc13 gjc13 Awaiting requested review from gjc13

@harsha-rajendran harsha-rajendran Awaiting requested review from harsha-rajendran

@hawk248 hawk248 Awaiting requested review from hawk248

@isiu-apple isiu-apple Awaiting requested review from isiu-apple

@jelderton jelderton Awaiting requested review from jelderton

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

@jmartinez-silabs jmartinez-silabs Awaiting requested review from jmartinez-silabs

@jtung-apple jtung-apple Awaiting requested review from jtung-apple

@kghost kghost Awaiting requested review from kghost

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@rgoliver rgoliver Awaiting requested review from rgoliver

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@tcarmelveilleux tcarmelveilleux Awaiting requested review from tcarmelveilleux

@tecimovic tecimovic Awaiting requested review from tecimovic

@turon turon Awaiting requested review from turon

@vijs vijs Awaiting requested review from vijs

@vivien-apple vivien-apple Awaiting requested review from vivien-apple

@wbschiller wbschiller Awaiting requested review from wbschiller

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

@yunhanw-google yunhanw-google Awaiting requested review from yunhanw-google

Assignees
No one assigned
Labels
controller review - approved sve
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Crash when accessory reboots during pairing kSessionEstablishmentTimeout is redundant
3 participants
@bzbarsky-apple @msandstedt @woody-apple