Skip to content

Commit

Permalink
Allow SetGroupDataProvider(nullptr) for resource mgmt (#17408)
Browse files Browse the repository at this point in the history
- Users implementing clean system shutdown must ensure that
  once they release their GroupDataProvider, the implementation
  does not dangle. After proper shutdown, GetGroupDataProvider()
  will never be called again, so it's fine to set it to nullptr.
  If it is indeed called, then next access will crash will null
  pointer dereference, which is more deterministic than use-after-free
  that would occur today.

- Allow setting GroupDataProvider singleton to nullptr

Testing done:
- Unit tests pass
- Integration tests pass
  • Loading branch information
tcarmelveilleux authored Apr 15, 2022
1 parent 737b702 commit 0680f21
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 23 deletions.
6 changes: 3 additions & 3 deletions src/credentials/GroupDataProvider.h
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ inline CHIP_ERROR SetSingleIpkEpochKey(GroupDataProvider * provider, FabricIndex
*
* Callers have to externally synchronize usage of this function.
*
* @return The global Group Data Provider. Assume never null.
* @return The global Group Data Provider
*/
GroupDataProvider * GetGroupDataProvider();

Expand All @@ -409,9 +409,9 @@ GroupDataProvider * GetGroupDataProvider();
*
* Callers have to externally synchronize usage of this function.
*
* If the `provider` is nullptr, no change is done.
* The `provider` can be set to nullptr if the owner is done with it fully.
*
* @param[in] provider the Group Data Provider
* @param[in] provider pointer to the Group Data Provider global isntance to use
*/
void SetGroupDataProvider(GroupDataProvider * provider);

Expand Down
21 changes: 1 addition & 20 deletions src/credentials/GroupDataProviderImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2015,33 +2015,14 @@ GroupDataProvider * gGroupsProvider = nullptr;

} // namespace

/**
* Instance getter for the global GroupDataProvider.
*
* Callers have to externally synchronize usage of this function.
*
* @return The global device attestation credentials provider. Assume never null.
*/
GroupDataProvider * GetGroupDataProvider()
{
return gGroupsProvider;
}

/**
* Instance setter for the global GroupDataProvider.
*
* Callers have to externally synchronize usage of this function.
*
* If the `provider` is nullptr, no change is done.
*
* @param[in] provider the GroupDataProvider to start returning with the getter
*/
void SetGroupDataProvider(GroupDataProvider * provider)
{
if (provider)
{
gGroupsProvider = provider;
}
gGroupsProvider = provider;
}

} // namespace Credentials
Expand Down

0 comments on commit 0680f21

Please sign in to comment.