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

Don't use EmberAfPluginDoorLockUserInfo if findUserIndexByCredential failed. #23467

Merged
merged 1 commit into from
Nov 9, 2022
Merged

Don't use EmberAfPluginDoorLockUserInfo if findUserIndexByCredential failed. #23467

merged 1 commit into from
Nov 9, 2022

Conversation

bzbarsky-apple
Copy link
Contributor

A few changes here:

  • Initialize the user status in EmberAfPluginDoorLockUserInfo to Available, to represent no user.
  • If we fail to find a user for the given PIN, don't include a bogus user index and credential index in the operation error event we send.
  • If we fail to find a user for the given PIN, send InvalidCredential as the operation error, not Unspecified.

@bzbarsky-apple
Don't use EmberAfPluginDoorLockUserInfo if findUserIndexByCredential …
1721d0c 
…failed.

A few changes here:

* Initialize the user status in EmberAfPluginDoorLockUserInfo to Available, to
  represent no user.
* If we fail to find a user for the given PIN, don't include a bogus user index
  and credential index in the operation error event we send.
* If we fail to find a user for the given PIN, send InvalidCredential as the
  operation error, not Unspecified.
@github-actions github-actions bot added the app label Nov 3, 2022
@github-actions
Copy link

github-actions bot commented Nov 3, 2022

PR #23467: Size comparison from 5bf3e3f to 1721d0c

Increases (7 builds for k32w, nrfconnect, qpg, telink)
platform target config section 5bf3e3f 1721d0c change % change
k32w lock k32w0+release (read/write) 633468 633516 48 0.0
.text 553060 553108 48 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1188191 1188303 112 0.0
rodata 144836 144916 80 0.1
text 816996 817026 30 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1167475 1167651 176 0.0
rodata 136576 136656 80 0.1
text 805334 805420 86 0.0
qpg lock-app qpg6105+debug (read/write) 1114760 1114848 88 0.0
.text 561856 561944 88 0.0
telink all-clusters-app tlsr9518adk80d (read/write) 937348 937480 132 0.0
text 648768 648820 52 0.0
all-clusters-minimal-app tlsr9518adk80d (read/write) 875156 875304 148 0.0
text 612608 612672 64 0.0
light-switch-app tlsr9518adk80d text 554266 554268 2 0.0
Decreases (2 builds for bl702, qpg)
platform target config section 5bf3e3f 1721d0c change % change
bl702 lighting-app bl702 .debug_info 38070160 38070159 -1 -0.0
.text 958000 957998 -2 -0.0
qpg lighting-app qpg6105+debug (read/write) 1149968 1149960 -8 -0.0
.text 597068 597060 -8 -0.0
Full report (16 builds for bl702, k32w, linux, nrfconnect, qpg, telink)
platform target config section 5bf3e3f 1721d0c change % change
bl702 lighting-app bl702 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1189595 1189595 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 67134 67134 0 0.0
.bss_psram 29696 29696 0 0.0
.comment 48 48 0 0.0
.data 4280 4280 0 0.0
.debug_abbrev 1513290 1513290 0 0.0
.debug_aranges 132064 132064 0 0.0
.debug_frame 484452 484452 0 0.0
.debug_info 38070160 38070159 -1 -0.0
.debug_line 5263845 5263845 0 0.0
.debug_loc 3366772 3366772 0 0.0
.debug_ranges 358120 358120 0 0.0
.debug_str 3451668 3451668 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 144 144 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 116680 116680 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 566021 566021 0 0.0
.symtab 172000 172000 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 958000 957998 -2 -0.0
bl702+rpc 0 0 0 0.0
(read only) 3262 3262 0 0.0
(read/write) 1285539 1285539 0 0.0
.bleromro 6296 6296 0 0.0
.bleromrw 124 124 0 0.0
.boot2 688 688 0 0.0
.bss 75166 75166 0 0.0
.bss_psram 29936 29936 0 0.0
.comment 48 48 0 0.0
.data 4816 4816 0 0.0
.debug_abbrev 1650704 1650704 0 0.0
.debug_aranges 139568 139568 0 0.0
.debug_frame 509824 509824 0 0.0
.debug_info 41986266 41986266 0 0.0
.debug_line 5639579 5639579 0 0.0
.debug_loc 3559599 3559599 0 0.0
.debug_ranges 380920 380920 0 0.0
.debug_str 3847796 3847796 0 0.0
.hbn 509 509 0 0.0
.hbn_noinit 260 260 0 0.0
.init 342 342 0 0.0
.init_array 160 160 0 0.0
.psram 0 0 0 0.0
.riscv.attributes 47 47 0 0.0
.rodata 130088 130088 0 0.0
.rsvd 3188 3188 0 0.0
.shstrtab 293 293 0 0.0
.stack 2048 2048 0 0.0
.strtab 625194 625194 0 0.0
.symtab 189808 189808 0 0.0
.tcm_data 36 36 0 0.0
.tcmcode 3262 3262 0 0.0
.text 1031704 1031704 0 0.0
k32w contact k32w0+release (read/write) 661676 661676 0 0.0
.bss 77040 77040 0 0.0
.data 2104 2104 0 0.0
.text 563420 563420 0 0.0
light k32w0+release (read/write) 672008 672008 0 0.0
.bss 74840 74840 0 0.0
.data 2060 2060 0 0.0
.text 592380 592380 0 0.0
lock k32w0+release (read/write) 633468 633516 48 0.0
.bss 75600 75600 0 0.0
.data 2080 2080 0 0.0
.text 553060 553108 48 0.0
linux chip-tool-ipv6only arm64 (read only) 10436348 10436348 0 0.0
(read/write) 706465 706465 0 0.0
.bss 33953 33953 0 0.0
.data 2768 2768 0 0.0
.data.rel.ro 650720 650720 0 0.0
.dynamic 560 560 0 0.0
.got 13928 13928 0 0.0
.init 24 24 0 0.0
.init_array 208 208 0 0.0
.rodata 518220 518220 0 0.0
.text 8262292 8262292 0 0.0
thermostat-no-ble arm64 (read only) 2392844 2392844 0 0.0
(read/write) 143665 143665 0 0.0
.bss 55377 55377 0 0.0
.data 1816 1816 0 0.0
.data.rel.ro 77280 77280 0 0.0
.dynamic 560 560 0 0.0
.got 5184 5184 0 0.0
.init 24 24 0 0.0
.init_array 440 440 0 0.0
.rodata 144548 144548 0 0.0
.text 2005296 2005296 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1188191 1188303 112 0.0
bss 145301 145301 0 0.0
rodata 144836 144916 80 0.1
text 816996 817026 30 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1167475 1167651 176 0.0
bss 144528 144528 0 0.0
rodata 136576 136656 80 0.1
text 805334 805420 86 0.0
qpg lighting-app qpg6105+debug (read/write) 1149968 1149960 -8 -0.0
.bss 110612 110612 0 0.0
.data 848 848 0 0.0
.text 597068 597060 -8 -0.0
lock-app qpg6105+debug (read/write) 1114760 1114848 88 0.0
.bss 106388 106388 0 0.0
.data 836 836 0 0.0
.text 561856 561944 88 0.0
telink all-clusters-app tlsr9518adk80d (read/write) 937348 937480 132 0.0
bss 81632 81632 0 0.0
noinit 43440 43440 0 0.0
text 648768 648820 52 0.0
all-clusters-minimal-app tlsr9518adk80d (read/write) 875156 875304 148 0.0
bss 80856 80856 0 0.0
noinit 43440 43440 0 0.0
text 612608 612672 64 0.0
light-switch-app tlsr9518adk80d (read/write) 790820 790820 0 0.0
bss 72480 72480 0 0.0
noinit 43520 43520 0 0.0
text 554266 554268 2 0.0
lighting-app tlsr9518adk80d (read/write) 875052 875052 0 0.0
bss 82248 82248 0 0.0
noinit 43520 43520 0 0.0
text 624254 624254 0 0.0
ota-requestor-app tlsr9518adk80d (read/write) 804952 804952 0 0.0
bss 74052 74052 0 0.0
noinit 43520 43520 0 0.0
text 565286 565286 0 0.0

@andy31415 andy31415 merged commit 78b6137 into project-chip:master Nov 9, 2022
@bzbarsky-apple bzbarsky-apple deleted the door-lock-fix-uninitialized-read branch November 9, 2022 13:11
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
@bzbarsky-apple @adbridge
Don't use EmberAfPluginDoorLockUserInfo if findUserIndexByCredential …
2768825 
…failed. (project-chip#23467)

A few changes here:

* Initialize the user status in EmberAfPluginDoorLockUserInfo to Available, to
  represent no user.
* If we fail to find a user for the given PIN, don't include a bogus user index
  and credential index in the operation error event we send.
* If we fail to find a user for the given PIN, send InvalidCredential as the
  operation error, not Unspecified.
adbridge pushed a commit to ARM-software/connectedhomeip that referenced this pull request Nov 18, 2022
@bzbarsky-apple @adbridge
Don't use EmberAfPluginDoorLockUserInfo if findUserIndexByCredential …
acd28a0 
…failed. (project-chip#23467)

A few changes here:

* Initialize the user status in EmberAfPluginDoorLockUserInfo to Available, to
  represent no user.
* If we fail to find a user for the given PIN, don't include a bogus user index
  and credential index in the operation error event we send.
* If we fail to find a user for the given PIN, send InvalidCredential as the
  operation error, not Unspecified.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@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

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

@gjc13 gjc13 Awaiting requested review from gjc13

@harimau-qirex harimau-qirex Awaiting requested review from harimau-qirex

@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

@kpschoedel kpschoedel Awaiting requested review from kpschoedel

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@rgoliver rgoliver Awaiting requested review from rgoliver

@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

@tehampson tehampson Awaiting requested review from tehampson

@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

@woody-apple woody-apple Awaiting requested review from woody-apple

@xylophone21 xylophone21 Awaiting requested review from xylophone21

@yufengwangca yufengwangca Awaiting requested review from yufengwangca

Assignees
No one assigned
Labels
app fast track review - pending
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bzbarsky-apple @andy31415