Fix attribute reads into a large buffer to not read random memory.

[bzbarsky-apple]

When reading an attribute value into a large buffer, we would read the
number of bytes that can fit in the buffer, not the number of bytes
the attribute value actually takes up. This would cause us to read
whatever memory came after the attribute value.

Also fixes some issues in typeSensitiveMemCopy when trying to
read a string-valued attribute into a buffer that's not even big
enough to fit the length value of the string.

Fixes #4371

Problem

We end up reading out-of-bounds memory when doing attribute reporting.

Summary of Changes

Fix typeSensitiveMemCopy to not read memory it should not.

[github-actions]

Size increase report for "esp32-example-build" from be3279a

File	Section	File	VM
chip-all-clusters-app.elf	.flash.text	28	28

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_loc,0,120
.debug_ranges,0,32
.debug_str,0,28
.flash.text,28,28
.debug_line,0,26
.debug_info,0,4
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,-2

[github-actions]

Size increase report for "nrfconnect-example-build" from be3279a

File	Section	File	VM
chip-lighting.elf	text	24	24
chip-lighting.elf	shell_root_cmds_sections	-8	-8
chip-lock.elf	text	24	24
chip-lock.elf	shell_root_cmds_sections	-8	-8

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-lighting.elf and ./pull_artifact/chip-lighting.elf:

sections,vmsize,filesize
text,24,24
.debug_loc,0,19
.debug_info,0,18
.debug_str,0,17
shell_root_cmds_sections,-8,-8
.debug_line,0,-14

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
text,24,24
.debug_info,0,18
.debug_str,0,17
.debug_loc,0,15
shell_root_cmds_sections,-8,-8
.debug_line,0,-10

[andy31415]

why is the caller buffer size ignored when write is set? I would expect at least a min(readLength, am-size) and I think we should log on mismatch to catch logic errors.

Generally I have complains about this method to start with: a buffer length that seems to be only relevant on read (called readLength) but method used on both read and write and a direction parameter when the caller could just have flipped src and destination.

I generally would apply the test 'Can one clearly in one sentence describe what a method does, generally without 'IF' or conditionals?'. I think this method fails hard because it has a 'write' parameter that affects behaviour as well as special conditions for readLength.

Could we fix up this method and make it somehow easier to use?

[bzbarsky-apple]

why is the caller buffer size ignored when write is set?

Apart from that being the documented behavior of emAfReadOrWriteAttribute and all callers who set write passing 0 for the size? I'm afraid I don't know why the setup is the way it is; that's a question for @selissia or @tecimovic.

Could we fix up this method and make it somehow easier to use?

Yes, we could, and I agree we should. We'd need to audit all consumers to see what they are doing with readLength; the comments suggest that there are legacy consumers that pass 0 for reads as well....

I'm happy to do that, but it might be a few days before I can pull enough time off of spec stuff to do it. I'm happy to land this in the meantime to unblock your ASan changes and do the larger semantic-changing refactoring in a followup, or expand the scope of this PR, as long as you're OK with the corresponding lag. Please let me know.

[andy31415]

created #4394 to track improvement in buffer handling within this code.

Is this generated code? if so, how would one go about on changing the code and updating?

[bzbarsky-apple]

Just to close the loop on this: this is not generated code.

[rwalker-apple]

@jelderton @saurabhst @hawk248 @BroderickCarlin ?