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 unbounded stack in emberAfPrintBuffer() #4633

Merged
merged 4 commits into from
Feb 3, 2021

Conversation

kpschoedel
Copy link
Contributor

Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

Summary of Changes

Print long buffers in multiple segments.

Fixes #3662 - Unbounded stack in emberAfPrintBuffer()

#### Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

#### Summary of Changes

Print long buffers in multiple segments.

Fixes project-chip#3662 - Unbounded stack in emberAfPrintBuffer()
// TODO: issue #3662 - Unbounded stack in emberAfPrintBuffer()
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstack-usage="

void emberAfPrintBuffer(int category, const uint8_t * buffer, uint16_t length, bool withSpace)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way we could unit test this? Maybe add a TODO PR (I think I wanted to do app/util unit tests before and failed, so it does not seem fair to ask you do do it as part of this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added TODO. I did verify the change out-of-tree with a #define emberAfPrint(c,...) printf(__VA_ARGS__).

@github-actions
Copy link

github-actions bot commented Feb 3, 2021

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

File Section File VM
chip-all-clusters-app.elf .flash.text 16 16
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_line,0,220
.debug_info,0,184
.debug_str,0,64
.debug_abbrev,0,54
.flash.text,16,16
.debug_ranges,0,-24
.debug_loc,0,-150


@github-actions
Copy link

github-actions bot commented Feb 3, 2021

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

File Section File VM
chip-lock.elf shell_root_cmds_sections 4 4
chip-lock.elf text -4 -4
chip-lighting.elf shell_root_cmds_sections 4 4
chip-lighting.elf text -4 -4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,2291
.debug_line,0,486
.debug_abbrev,0,327
.debug_str,0,52
.debug_loc,0,28
shell_root_cmds_sections,4,4
text,-4,-4
.debug_frame,0,-8

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
.debug_info,0,2291
.debug_line,0,486
.debug_abbrev,0,327
.debug_str,0,52
.debug_loc,0,36
shell_root_cmds_sections,4,4
text,-4,-4
.debug_frame,0,-8


@woody-apple woody-apple added p1 priority 1 work security labels Feb 3, 2021
@woody-apple woody-apple added this to the V0.7 milestone Feb 3, 2021
@woody-apple woody-apple merged commit 1cfac2d into project-chip:master Feb 3, 2021
@kpschoedel kpschoedel deleted the x3662-stack branch February 3, 2021 19:27
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this pull request Feb 3, 2021
* Fix unbounded stack in emberAfPrintBuffer()

#### Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

#### Summary of Changes

Print long buffers in multiple segments.

Fixes project-chip#3662 - Unbounded stack in emberAfPrintBuffer()

* Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size

* TODO

* casts
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 3, 2021
* Fix unbounded stack in emberAfPrintBuffer()

#### Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

#### Summary of Changes

Print long buffers in multiple segments.

Fixes project-chip#3662 - Unbounded stack in emberAfPrintBuffer()

* Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size

* TODO

* casts
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 3, 2021
* Fix unbounded stack in emberAfPrintBuffer()

#### Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

#### Summary of Changes

Print long buffers in multiple segments.

Fixes project-chip#3662 - Unbounded stack in emberAfPrintBuffer()

* Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size

* TODO

* casts
austinh0 pushed a commit to austinh0/connectedhomeip that referenced this pull request Feb 4, 2021
* Fix unbounded stack in emberAfPrintBuffer()

#### Problem

emberAfPrintBuffer() allocates stack space based on an argument, with
no upper bound. This could lead to stack exhaustion on small devices.

#### Summary of Changes

Print long buffers in multiple segments.

Fixes project-chip#3662 - Unbounded stack in emberAfPrintBuffer()

* Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size

* TODO

* casts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app p1 priority 1 work security
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unbounded stack in emberAfPrintBuffer()
4 participants