8000 Seg fault in src/app/util/attribute-storage.cpp · Issue #22272 · project-chip/connectedhomeip · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Seg fault in src/app/util/attribute-storage.cpp #22272

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

Closed
latch-danielkneip opened this issue Aug 30, 2022 · 2 comments · Fixed by #22275
Closed

Seg fault in src/app/util/attribute-storage.cpp #22272

latch-danielkneip opened this issue Aug 30, 2022 · 2 comments · Fixed by #22275
Labels

Comments

@latch-danielkneip
Copy link
Contributor

Problem

We’ve run into a seg fault in src/app/util/attribute-storage.cpp when emAfEndpoints[ep].endpointType is 0 and emberAfFindClusterInType() is called from emberAfFindClusterIncludingDisabledEndpoints(). Adding emAfEndpoints[ep].endpointType != nullptr before the call allows us to function normally. However, I’m concerned there may be a deeper issue.

The seg fault occurs after issuing the following command to dynamic endpoint 4 that represents a bridged dimmer:

./chip-tool onoff toggle 1 4

Proposed Solution

diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp
index 7e8bad312..7d3124d30 100644
--- a/src/app/util/attribute-storage.cpp
+++ b/src/app/util/attribute-storage.cpp
@@ -796,7 +796,7 @@ const EmberAfCluster * emberAfFindClusterIncludingDisabledEndpoints(EndpointId e
                                                                     EmberAfClusterMask mask)
 {
     uint16_t ep = emberAfIndexFromEndpointIncludingDisabledEndpoints(endpoint);
-    if (ep < MAX_ENDPOINT_COUNT)
+    if (ep < MAX_ENDPOINT_COUNT && emAfEndpoints[ep].endpointType != nullptr)
     {
         return emberAfFindClusterInType(emAfEndpoints[ep].endpointType, clusterId, mask);
     }
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Aug 30, 2022
There are some code paths that loop over all endpoint indices (including ones
that do not have an endpoint defined yet) and then use the endpoint id to look
up endpoint indices.  Make sure we don't claim an endpoint index for the "not an
endpoint" endpoint id.

Also fixes findClusterEndpointIndex to check for the invalid endpoint id before
working with it, since that means the endpoint index it's looking at does not
correspond to a defined endpoint.

Fixes project-chip#22272
@bzbarsky-apple
Copy link
Contributor

If we don't fix this for 1.0, I expect we will have common crashes on any app that allocates some dynamic endpoints but does not actually have endpoints defined for all of them (e.g. any bridge that is not maxed out in terms of the number of devices it's bridging).

@andy31415
Copy link
Contributor

Master patch acceptable: segfault fix.

bzbarsky-apple added a commit that referenced this issue Aug 31, 2022
…22275)

There are some code paths that loop over all endpoint indices (including ones
that do not have an endpoint defined yet) and then use the endpoint id to look
up endpoint indices.  Make sure we don't claim an endpoint index for the "not an
endpoint" endpoint id.

Also fixes findClusterEndpointIndex to check for the invalid endpoint id before
working with it, since that means the endpoint index it's looking at does not
correspond to a defined endpoint.

Fixes #22272
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this issue Sep 16, 2022
…roject-chip#22275)

There are some code paths that loop over all endpoint indices (including ones
that do not have an endpoint defined yet) and then use the endpoint id to look
up endpoint indices.  Make sure we don't claim an endpoint index for the "not an
endpoint" endpoint id.

Also fixes findClusterEndpointIndex to check for the invalid endpoint id before
working with it, since that means the endpoint index it's looking at does not
correspond to a defined endpoint.

Fixes project-chip#22272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
0