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

Unbounded stack in emberAfPrintBuffer() #3662

Closed
kpschoedel opened this issue Nov 4, 2020 · 1 comment · Fixed by #4633
Closed

Unbounded stack in emberAfPrintBuffer() #3662

kpschoedel opened this issue Nov 4, 2020 · 1 comment · Fixed by #4633
Labels
p1 priority 1 work security V1.0
Milestone
V1.0

Comments

@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.

Proposed Solution

Since this is used for logging, to dump data in hexadecimal, it may be sufficient to break a long argument into multiple lines.
@issue-label-bot
Copy link

Issue Label Bot is not confident enough to auto-label this issue. See dashboard for more details.

kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Nov 5, 2020
@kpschoedel
Enable -Wstack-usage on device builds
7acc910 
#### Problem

RAM on small devices is limited, so excesssive stack should not be used
without good reason. Stack allocation is also not easily checked at run
time so overflow is likely to cause crashes.

#### Summary of Changes

- Add a `-Wstack-usage` compiler flag on embedded builds. (For this
  purpose, an ‘embedded build’ is any that is not using a whitelisted
  non-embedded OS, so new platforms will have this enabled by default.)

  In this PR, the stack limit is set high enough that _only_ dynamically
  unbounded stack usage triggers it. The intent is to lower the limit
  in the future so that any unusually large stack requires whitelisting
  with justification in review.

- Replace most uses of dynamically unbounded stack.

- Filed separate issues for two remaining uses of dynamic stack:
    - project-chip#3662 emberAfPrintBuffer()
    - project-chip#3663 payloadBase41RepresentationWithTLV()

fixes project-chip#3505 Use "-Wstack-usage" for device builds
@kpschoedel kpschoedel mentioned this issue Nov 5, 2020
Merged
bzbarsky-apple pushed a commit that referenced this issue Nov 7, 2020
@kpschoedel
Enable -Wstack-usage on device builds (#3683)
2d17edb 
* Enable -Wstack-usage on device builds

#### Problem

RAM on small devices is limited, so excesssive stack should not be used
without good reason. Stack allocation is also not easily checked at run
time so overflow is likely to cause crashes.

#### Summary of Changes

- Add a `-Wstack-usage` compiler flag on embedded builds. (For this
  purpose, an ‘embedded build’ is any that is not using a whitelisted
  non-embedded OS, so new platforms will have this enabled by default.)

  In this PR, the stack limit is set high enough that _only_ dynamically
  unbounded stack usage triggers it. The intent is to lower the limit
  in the future so that any unusually large stack requires whitelisting
  with justification in review.

- Replace most uses of dynamically unbounded stack.

- Filed separate issues for two remaining uses of dynamic stack:
    - #3662 emberAfPrintBuffer()
    - #3663 payloadBase41RepresentationWithTLV()

fixes #3505 Use "-Wstack-usage" for device builds

* Fix dynamic-sized stack array in CHIPDeviceController.cpp

* Additional dynamic-stack-size fixes.

* Revert ManualSetupPayloadGenerator.cpp
@woody-apple woody-apple added this to the V1.0 milestone Jan 29, 2021
@kpschoedel kpschoedel mentioned this issue Feb 3, 2021
Merged
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Feb 3, 2021
@kpschoedel
Fix unbounded stack in emberAfPrintBuffer()
1101ecc 
#### 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()
woody-apple pushed a commit that referenced this issue Feb 3, 2021
@kpschoedel
Fix unbounded stack in emberAfPrintBuffer() (#4633)
1cfac2d 
* 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 #3662 - Unbounded stack in emberAfPrintBuffer()

* Use CHIP_DEVICE_CONFIG_LOG_MESSAGE_MAX_SIZE for buffer size

* TODO

* casts
kpschoedel added a commit to kpschoedel/connectedhomeip that referenced this issue Feb 3, 2021
@kpschoedel
Fix unbounded stack in emberAfPrintBuffer() (project-chip#4633)
d91b540 
* 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 issue Feb 3, 2021
@kpschoedel @austinh0
Fix unbounded stack in emberAfPrintBuffer() (project-chip#4633)
5620ae7 
* 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 issue Feb 3, 2021
@kpschoedel @austinh0
Fix unbounded stack in emberAfPrintBuffer() (project-chip#4633)
0335601 
* 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 issue Feb 4, 2021
@kpschoedel @austinh0
Fix unbounded stack in emberAfPrintBuffer() (project-chip#4633)
5a8db44 
* 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
Assignees
No one assigned
Labels
p1 priority 1 work security V1.0
Projects
[Performance] Flash + RAM usage
Archived in project
Milestone
V1.0
Development

Successfully merging a pull request may close this issue.

Fix unbounded stack in emberAfPrintBuffer() kpschoedel/connectedhomeip
2 participants
@woody-apple @kpschoedel