Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

samples: matter: Added nullptr checks after memory allocation #19974

Merged
merged 2 commits into from
Jan 20, 2025

Conversation

kkasperczyk-no
Copy link
Contributor

In few places there are missing nullptr checks, what can lead to using not allocated memory.

@kkasperczyk-no kkasperczyk-no requested a review from a team as a code owner January 17, 2025 12:53
@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 17, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jan 17, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 5

Inputs:

Sources:

sdk-nrf: PR head: fa1888eae4547b710c9d1ee364fe4d7bff8c51d2
matter: PR head: 7e94f1c336ed437c32d0dec80c72fa9f4f2ce3ee

more details

sdk-nrf:

PR head: fa1888eae4547b710c9d1ee364fe4d7bff8c51d2
merge base: 4389cc31d0f31cd2efe5aa851fcb65ffeaa3f30f
target head (main): 42aa59e7d9fc50a0eeb6c0e9af694928268b400f
Diff

matter:

PR head: 7e94f1c336ed437c32d0dec80c72fa9f4f2ce3ee
merge base: 36d3ddf55de6cb700e859af8a519253fa7dd7712
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (23)
modules
│  ├── lib
│  │  ├── matter
│  │  │  ├── config
│  │  │  │  ├── nrfconnect
│  │  │  │  │  ├── chip-module
│  │  │  │  │  │  ├── CMakeLists.txt
│  │  │  │  │  │  ├── Kconfig
│  │  │  │  │  │  │ Kconfig.defaults
samples
│  ├── matter
│  │  ├── common
│  │  │  ├── src
│  │  │  │  ├── binding
│  │  │  │  │  │ binding_handler.cpp
│  │  ├── light_bulb
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── light_switch
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  │  ├── src
│  │  │  │  │ shell_commands.cpp
│  │  ├── lock
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── smoke_co_alarm
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l10_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp.conf
│  │  ├── template
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l10_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp_internal.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  ├── thermostat
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
│  │  │  ├── src
│  │  │  │  ├── temperature_measurement
│  │  │  │  │  │ sensor.cpp
│  │  ├── window_covering
│  │  │  ├── boards
│  │  │  │  ├── nrf54l15dk_nrf54l15_cpuapp.conf
│  │  │  │  │ nrf54l15dk_nrf54l15_cpuapp_ns.conf
west.yml

Outputs:

Toolchain

Version:
Build docker image:

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain
  • ◻️ Build twister
  • ◻️ Integration tests
    • ◻️ test-fw-nrfconnect-chip
Disabled integration tests
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_mesh
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-boot
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_mosh
    • test-fw-nrfconnect-nrf-iot_positioning
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread
    • test-fw-nrfconnect-zigbee
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-sidewalk
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

Copy link
Member

@adigie adigie left a comment

Choose a reason for hiding this comment

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

There are a few more unchecked allocations in lambda captures that are later used in BindingHandler::OnInvokeCommandSucces (thermostat, light_bulb, matter_bridge). We can add check there.

In few places there are missing nullptr checks, what can
lead to using not allocated memory.

Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
@ArekBalysNordic ArekBalysNordic removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 17, 2025
Copy link
Contributor

@LuDuda LuDuda left a comment

Choose a reason for hiding this comment

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

Thanks Kamil it looks great 👍

Btw. Don't we need to increase some of the heap size? While we are now protected, the issue was triggered with normal condition.

@kkasperczyk-no kkasperczyk-no requested a review from a team as a code owner January 20, 2025 10:44
@github-actions github-actions bot added manifest changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. labels Jan 20, 2025
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jan 20, 2025

The following west manifest projects have changed revision in this Pull Request:

Name Old Revision New Revision Diff
matter nrfconnect/sdk-connectedhomeip@36d3ddf nrfconnect/sdk-connectedhomeip@7e94f1c (master) nrfconnect/sdk-connectedhomeip@36d3ddf5..7e94f1c3

All manifest checks OK

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@kkasperczyk-no
Copy link
Contributor Author

@adigie @ArekBalysNordic please take a look once again, as I've introduced some new configuration changes (also there is another PR linked)

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link.

Note: This comment is automatically posted by the Documentation Publish GitHub Action.

Removed the common mpsl stack size config to the Kconfig.defaults.
Additionally removed some configuration leftover for the 54L10.

Signed-off-by: Kamil Kasperczyk <kamil.kasperczyk@nordicsemi.no>
@NordicBuilder NordicBuilder removed the DNM label Jan 20, 2025
@ArekBalysNordic ArekBalysNordic removed the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jan 20, 2025
@ArekBalysNordic ArekBalysNordic merged commit 66ab9ae into nrfconnect:main Jan 20, 2025
14 of 15 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants