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

Ensure null-termination in Darwin DNS SD #16043

Merged
merged 5 commits into from
Mar 15, 2022
Merged

Ensure null-termination in Darwin DNS SD #16043

merged 5 commits into from
Mar 15, 2022

Conversation

lzgrablic02
Copy link
Contributor

@lzgrablic02 lzgrablic02 commented Mar 9, 2022 •
edited
Loading

Problem

What is being fixed? Examples:

Change overview

Ensure proper null-termination with CopyString.

Testing

How was this tested? (at least one bullet point required)

  • None.

@lzgrablic02
Ensure null-termination in Darwin DNS SD
2a96a2a 
Null-termination was not ensured after strncpy.
@restyled-io restyled-io bot mentioned this pull request Mar 9, 2022
Closed
bzbarsky-apple
bzbarsky-apple requested changes Mar 9, 2022
View reviewed changes
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect if we use Platform::CopyString the code will be clearer and we won't need the cstring header and the extra std:: on strcmp.

src/platform/Darwin/DnssdImpl.h Outdated Show resolved Hide resolved
src/platform/Darwin/DnssdImpl.h Outdated Show resolved Hide resolved
src/platform/Darwin/DnssdImpl.h Outdated Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Mar 9, 2022 •
edited
Loading

PR #16043: Size comparison from 190520d to 8bbee8a

Full report (26 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6, telink)
platform target config section 190520d 8bbee8a change % change
cyw30739 light cyw930739m2evb_01 (read/write) 598058 598058 0 0.0
.app_xip_area 505640 505640 0 0.0
.bss 75108 75108 0 0.0
.data 652 652 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 555926 555926 0 0.0
.app_xip_area 465044 465044 0 0.0
.bss 73612 73612 0 0.0
.data 616 616 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor cyw930739m2evb_01 (read/write) 566030 566030 0 0.0
.app_xip_area 465444 465444 0 0.0
.bss 82996 82996 0 0.0
.data 552 552 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 919872 919872 0 0.0
(read/write) 128264 128264 0 0.0
.bss 126224 126224 0 0.0
.data 2040 2040 0 0.0
.text 919864 919864 0 0.0
BRD4161A+rpc (read only) 948700 948700 0 0.0
(read/write) 144220 144220 0 0.0
.bss 142000 142000 0 0.0
.data 2220 2220 0 0.0
.text 948692 948692 0 0.0
window-app BRD4161A (read only) 852348 852348 0 0.0
(read/write) 126220 126220 0 0.0
.bss 124304 124304 0 0.0
.data 1916 1916 0 0.0
.text 852340 852340 0 0.0
esp32 all-clusters-app c3devkit (read only) 957736 957736 0 0.0
(read/write) 1393778 1393778 0 0.0
.dram0.bss 63576 63576 0 0.0
.dram0.data 14220 14220 0 0.0
.flash.rodata 197064 197064 0 0.0
.flash.text 957736 957736 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1012887 1012887 0 0.0
(read/write) 461028 461028 0 0.0
.dram0.bss 69096 69096 0 0.0
.dram0.data 34064 34064 0 0.0
.flash.rodata 226032 226032 0 0.0
.flash.text 1007503 1007503 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 694492 694492 0 0.0
.bss 77160 77160 0 0.0
.data 1924 1924 0 0.0
.text 609608 609608 0 0.0
lock k32w061+release (read/write) 695292 695292 0 0.0
.bss 77072 77072 0 0.0
.data 1964 1964 0 0.0
.text 610456 610456 0 0.0
linux all-clusters-app debug (read only) 2430321 2430321 0 0.0
(read/write) 145872 145872 0 0.0
.bss 60352 60352 0 0.0
.data 1360 1360 0 0.0
.data.rel.ro 78424 78424 0 0.0
.dynamic 592 592 0 0.0
.got 4176 4176 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 205637 205637 0 0.0
.text 2061698 2061698 0 0.0
bridge-app debug+rpc (read only) 1735493 1735493 0 0.0
(read/write) 92784 92784 0 0.0
.bss 47624 47624 0 0.0
.data 2016 2016 0 0.0
.data.rel.ro 38064 38064 0 0.0
.dynamic 592 592 0 0.0
.got 3928 3928 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 141868 141868 0 0.0
.text 1477653 1477653 0 0.0
chip-tool debug (read only) 10093045 10093045 0 0.0
(read/write) 358168 358168 0 0.0
.bss 25640 25640 0 0.0
.data 1152 1152 0 0.0
.data.rel.ro 325352 325352 0 0.0
.dynamic 608 608 0 0.0
.got 4776 4776 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 514196 514196 0 0.0
.text 8796165 8796165 0 0.0
chip-tool-ipv6only arm64 (read only) 9729052 9729052 0 0.0
(read/write) 475969 475969 0 0.0
.bss 43953 43953 0 0.0
.data 1200 1200 0 0.0
.data.rel.ro 371712 371712 0 0.0
.dynamic 560 560 0 0.0
.got 55304 55304 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 491908 491908 0 0.0
.text 8185956 8185956 0 0.0
door-lock-app debug (read only) 1949953 1949953 0 0.0
(read/write) 119296 119296 0 0.0
.bss 50656 50656 0 0.0
.data 1056 1056 0 0.0
.data.rel.ro 62184 62184 0 0.0
.dynamic 592 592 0 0.0
.got 4112 4112 0 0.0
.init 27 27 0 0.0
.init_array 664 664 0 0.0
.rodata 173756 173756 0 0.0
.text 1627554 1627554 0 0.0
lighting-app debug+rpc (read only) 2082121 2082121 0 0.0
(read/write) 124616 124616 0 0.0
.bss 51456 51456 0 0.0
.data 1536 1536 0 0.0
.data.rel.ro 66104 66104 0 0.0
.dynamic 608 608 0 0.0
.got 4144 4144 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 165948 165948 0 0.0
.text 1760514 1760514 0 0.0
ota-provider-app debug (read only) 1883193 1883193 0 0.0
(read/write) 114896 114896 0 0.0
.bss 50464 50464 0 0.0
.data 1320 1320 0 0.0
.data.rel.ro 57464 57464 0 0.0
.dynamic 608 608 0 0.0
.got 4368 4368 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 158963 158963 0 0.0
.text 1573506 1573506 0 0.0
ota-requestor-app debug (read only) 1910801 1910801 0 0.0
(read/write) 117864 117864 0 0.0
.bss 51584 51584 0 0.0
.data 1480 1480 0 0.0
.data.rel.ro 59368 59368 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 154924 154924 0 0.0
.text 1605362 1605362 0 0.0
shell debug (read only) 2396513 2396513 0 0.0
(read/write) 150088 150088 0 0.0
.bss 70472 70472 0 0.0
.data 864 864 0 0.0
.data.rel.ro 73064 73064 0 0.0
.dynamic 592 592 0 0.0
.got 4160 4160 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 205586 205586 0 0.0
.text 2034354 2034354 0 0.0
thermostat-no-ble arm64 (read only) 2178620 2178620 0 0.0
(read/write) 148817 148817 0 0.0
.bss 65393 65393 0 0.0
.data 1048 1048 0 0.0
.data.rel.ro 75160 75160 0 0.0
.dynamic 560 560 0 0.0
.got 4240 4240 0 0.0
.init 24 24 0 0.0
.init_array 352 352 0 0.0
.rodata 133116 133116 0 0.0
.text 1826912 1826912 0 0.0
tv-app debug (read only) 2622009 2622009 0 0.0
(read/write) 250360 250360 0 0.0
.bss 167520 167520 0 0.0
.data 3168 3168 0 0.0
.data.rel.ro 73624 73624 0 0.0
.dynamic 592 592 0 0.0
.got 4544 4544 0 0.0
.init 27 27 0 0.0
.init_array 880 880 0 0.0
.rodata 199293 199293 0 0.0
.text 2248626 2248626 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2351356 2351356 0 0.0
.bss 186212 186212 0 0.0
.data 5792 5792 0 0.0
.text 1313956 1313956 0 0.0
p6 all-clusters-app default (read/write) 2494216 2494216 0 0.0
.bss 119640 119640 0 0.0
.data 2680 2680 0 0.0
.text 1452480 1452480 0 0.0
light-app default (read/write) 2398296 2398296 0 0.0
.bss 113104 113104 0 0.0
.data 2536 2536 0 0.0
.text 1356560 1356560 0 0.0
lock-app default (read/write) 2361856 2361856 0 0.0
.bss 112848 112848 0 0.0
.data 2496 2496 0 0.0
.text 1320120 1320120 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 888882 888882 0 0.0
bss 86968 86968 0 0.0
noinit 37160 37160 0 0.0
text 628184 628184 0 0.0

@lzgrablic02 @bzbarsky-apple
Apply suggestions from code review
52a8920 
Use CopyString instead of strncpy.

Co-authored-by: Boris Zbarsky <[email protected]>
@lzgrablic02
Copy link
Contributor Author

I suspect if we use Platform::CopyString the code will be clearer and we won't need the cstring header and the extra std:: on strcmp.

Definitely clearer. Doesn't affect the need for cstring and scope that was missing on strcmp though.

@restyled-io restyled-io bot mentioned this pull request Mar 15, 2022
Closed
@lzgrablic02
Use C header for strcmp instead of C++
720091a 
In src/platform/Darwin/DnssdImpl.h.
@github-actions
Copy link

github-actions bot commented Mar 15, 2022 •
edited
Loading

PR #16043: Size comparison from 5370ccb to 720091a

Full report (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 5370ccb 720091a change % change
cyw30739 light cyw930739m2evb_01 (read/write) 602562 602562 0 0.0
.app_xip_area 509668 509668 0 0.0
.bss 75648 75648 0 0.0
.data 596 596 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
lock cyw930739m2evb_01 (read/write) 560422 560422 0 0.0
.app_xip_area 469056 469056 0 0.0
.bss 74152 74152 0 0.0
.data 560 560 0 0.0
.rodata 0 0 0 0.0
.text 0 0 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 570490 570490 0 0.0
.app_xip_area 469476 469476 0 0.0
.bss 83456 83456 0 0.0
.data 520 520 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read only) 919700 919700 0 0.0
(read/write) 128752 128752 0 0.0
.bss 126744 126744 0 0.0
.data 2008 2008 0 0.0
.text 919692 919692 0 0.0
BRD4161A+rpc (read only) 948488 948488 0 0.0
(read/write) 144712 144712 0 0.0
.bss 142520 142520 0 0.0
.data 2188 2188 0 0.0
.text 948480 948480 0 0.0
window-app BRD4161A (read only) 850456 850456 0 0.0
(read/write) 126712 126712 0 0.0
.bss 124848 124848 0 0.0
.data 1864 1864 0 0.0
.text 850448 850448 0 0.0
esp32 all-clusters-app c3devkit (read only) 961466 961466 0 0.0
(read/write) 1394978 1394978 0 0.0
.dram0.bss 64056 64056 0 0.0
.dram0.data 14188 14188 0 0.0
.flash.rodata 197816 197816 0 0.0
.flash.text 961466 961466 0 0.0
.iram0.text 62016 62016 0 0.0
m5stack (read only) 1016983 1016983 0 0.0
(read/write) 462188 462188 0 0.0
.dram0.bss 69576 69576 0 0.0
.dram0.data 34016 34016 0 0.0
.flash.rodata 226760 226760 0 0.0
.flash.text 1011599 1011599 0 0.0
.iram0.text 123107 123107 0 0.0
k32w light k32w061+release (read/write) 699324 699324 0 0.0
.bss 77624 77624 0 0.0
.data 1888 1888 0 0.0
.text 614012 614012 0 0.0
lock k32w061+release (read/write) 699628 699628 0 0.0
.bss 77616 77616 0 0.0
.data 1908 1908 0 0.0
.text 614304 614304 0 0.0
linux all-clusters-app debug (read only) 2465449 2465449 0 0.0
(read/write) 146352 146352 0 0.0
.bss 60640 60640 0 0.0
.data 1264 1264 0 0.0
.data.rel.ro 78728 78728 0 0.0
.dynamic 592 592 0 0.0
.got 4168 4168 0 0.0
.init 27 27 0 0.0
.init_array 936 936 0 0.0
.rodata 210757 210757 0 0.0
.text 2091458 2091458 0 0.0
bridge-app debug+rpc (read only) 1750077 1750077 0 0.0
(read/write) 92912 92912 0 0.0
.bss 47752 47752 0 0.0
.data 1952 1952 0 0.0
.data.rel.ro 38128 38128 0 0.0
.dynamic 592 592 0 0.0
.got 3920 3920 0 0.0
.init 27 27 0 0.0
.init_array 544 544 0 0.0
.rodata 143372 143372 0 0.0
.text 1491157 1491157 0 0.0
chip-tool debug (read only) 10095789 10095789 0 0.0
(read/write) 357680 357680 0 0.0
.bss 25696 25696 0 0.0
.data 1072 1072 0 0.0
.data.rel.ro 324864 324864 0 0.0
.dynamic 608 608 0 0.0
.got 4776 4776 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 515189 515189 0 0.0
.text 8799349 8799349 0 0.0
chip-tool-ipv6only arm64 (read only) 9728724 9728724 0 0.0
(read/write) 475441 475441 0 0.0
.bss 44017 44017 0 0.0
.data 1128 1128 0 0.0
.data.rel.ro 371232 371232 0 0.0
.dynamic 560 560 0 0.0
.got 55264 55264 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 492252 492252 0 0.0
.text 8186596 8186596 0 0.0
door-lock-app debug (read only) 1984609 1984609 0 0.0
(read/write) 119776 119776 0 0.0
.bss 50944 50944 0 0.0
.data 992 992 0 0.0
.data.rel.ro 62456 62456 0 0.0
.dynamic 592 592 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 664 664 0 0.0
.rodata 178460 178460 0 0.0
.text 1657250 1657250 0 0.0
lighting-app debug+rpc (read only) 2116185 2116185 0 0.0
(read/write) 125160 125160 0 0.0
.bss 51776 51776 0 0.0
.data 1472 1472 0 0.0
.data.rel.ro 66408 66408 0 0.0
.dynamic 608 608 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 736 736 0 0.0
.rodata 171004 171004 0 0.0
.text 1789266 1789266 0 0.0
ota-provider-app debug (read only) 1919673 1919673 0 0.0
(read/write) 115568 115568 0 0.0
.bss 50848 50848 0 0.0
.data 1288 1288 0 0.0
.data.rel.ro 57816 57816 0 0.0
.dynamic 608 608 0 0.0
.got 4360 4360 0 0.0
.init 27 27 0 0.0
.init_array 616 616 0 0.0
.rodata 164491 164491 0 0.0
.text 1604178 1604178 0 0.0
ota-requestor-app debug (read only) 1949473 1949473 0 0.0
(read/write) 118568 118568 0 0.0
.bss 51840 51840 0 0.0
.data 1416 1416 0 0.0
.data.rel.ro 59880 59880 0 0.0
.dynamic 592 592 0 0.0
.got 4152 4152 0 0.0
.init 27 27 0 0.0
.init_array 640 640 0 0.0
.rodata 160516 160516 0 0.0
.text 1637730 1637730 0 0.0
shell debug (read only) 2411609 2411609 0 0.0
(read/write) 150184 150184 0 0.0
.bss 70600 70600 0 0.0
.data 784 784 0 0.0
.data.rel.ro 73088 73088 0 0.0
.dynamic 592 592 0 0.0
.got 4152 4152 0 0.0
.init 27 27 0 0.0
.init_array 920 920 0 0.0
.rodata 207538 207538 0 0.0
.text 2047922 2047922 0 0.0
thermostat-no-ble arm64 (read only) 2207484 2207484 0 0.0
(read/write) 149377 149377 0 0.0
.bss 65649 65649 0 0.0
.data 1024 1024 0 0.0
.data.rel.ro 75368 75368 0 0.0
.dynamic 560 560 0 0.0
.got 4352 4352 0 0.0
.init 24 24 0 0.0
.init_array 360 360 0 0.0
.rodata 136828 136828 0 0.0
.text 1851344 1851344 0 0.0
tv-app debug (read only) 2656449 2656449 0 0.0
(read/write) 250776 250776 0 0.0
.bss 167840 167840 0 0.0
.data 3104 3104 0 0.0
.data.rel.ro 73800 73800 0 0.0
.dynamic 592 592 0 0.0
.got 4536 4536 0 0.0
.init 27 27 0 0.0
.init_array 880 880 0 0.0
.rodata 204277 204277 0 0.0
.text 2278114 2278114 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2352692 2352692 0 0.0
.bss 186652 186652 0 0.0
.data 5752 5752 0 0.0
.text 1315292 1315292 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1154071 1154071 0 0.0
bss 146724 146724 0 0.0
rodata 145272 145272 0 0.0
text 786916 786916 0 0.0
p6 all-clusters-app default (read/write) 2492136 2492136 0 0.0
.bss 120080 120080 0 0.0
.data 2632 2632 0 0.0
.text 1450400 1450400 0 0.0
light-app default (read/write) 2395984 2395984 0 0.0
.bss 113544 113544 0 0.0
.data 2488 2488 0 0.0
.text 1354248 1354248 0 0.0
lock-app default (read/write) 2359528 2359528 0 0.0
.bss 113288 113288 0 0.0
.data 2448 2448 0 0.0
.text 1317792 1317792 0 0.0
telink lighting-app tlsr9518adk80d (read/write) 893762 893762 0 0.0
bss 87432 87432 0 0.0
noinit 37160 37160 0 0.0
text 631860 631860 0 0.0

@bzbarsky-apple bzbarsky-apple merged commit 046100b into project-chip:master Mar 15, 2022
@lzgrablic02 lzgrablic02 deleted the dnssd branch March 15, 2022 22:39
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this pull request Apr 14, 2022
@lzgrablic02 @bzbarsky-apple @andrei-menzopol
Ensure null-termination in Darwin DNS SD (project-chip#16043)
62a9661 
* Ensure null-termination in Darwin DNS SD

Null-termination was not ensured after strncpy.

* Rearrange style

* Apply suggestions from code review

Use CopyString instead of strncpy.

Co-authored-by: Boris Zbarsky <[email protected]>

* Add missing header for Darwin DnssdImpl

* Use C header for strcmp instead of C++

In src/platform/Darwin/DnssdImpl.h.

Co-authored-by: Boris Zbarsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bzbarsky-apple bzbarsky-apple bzbarsky-apple approved these changes

@tcarmelveilleux tcarmelveilleux tcarmelveilleux approved these changes

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

@austinh0 austinh0 Awaiting requested review from austinh0

@balducci-apple balducci-apple Awaiting requested review from balducci-apple

@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

@erjiaqing erjiaqing Awaiting requested review from erjiaqing

@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

@hawk248 hawk248 Awaiting requested review from hawk248

@holbrookt holbrookt Awaiting requested review from holbrookt

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

@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

@lazarkov lazarkov Awaiting requested review from lazarkov

@LuDuda LuDuda Awaiting requested review from LuDuda

@mlepage-google mlepage-google Awaiting requested review from mlepage-google

@mrjerryjohns mrjerryjohns Awaiting requested review from mrjerryjohns

@msandstedt msandstedt Awaiting requested review from msandstedt

@mspang mspang Awaiting requested review from mspang

@robszewczyk robszewczyk Awaiting requested review from robszewczyk

@sagar-apple sagar-apple Awaiting requested review from sagar-apple

@saurabhst saurabhst Awaiting requested review from saurabhst

@selissia selissia Awaiting requested review from selissia

@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

@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
darwin platform review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Potentially unterminated strings in DNS SD
3 participants
@lzgrablic02 @tcarmelveilleux @bzbarsky-apple