From 33303018795c2ae379d5c856d274e870785b0b4e Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Thu, 3 Oct 2024 10:02:15 -0400 Subject: [PATCH] Fetch next occupied credential index earlier in getCredentialStatusCommandHandler. (#35868) We want do to that before we actually fetch the credential we are trying to get the status of, since determining the next occupied credential index might stomp on data returned by emberAfPluginDoorLockGetCredential. --- .../door-lock-server/door-lock-server.cpp | 37 +++++++++++++------ .../door-lock-server/door-lock-server.h | 3 +- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/app/clusters/door-lock-server/door-lock-server.cpp b/src/app/clusters/door-lock-server/door-lock-server.cpp index d0574958c953aa..f58307d917b924 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.cpp +++ b/src/app/clusters/door-lock-server/door-lock-server.cpp @@ -860,10 +860,27 @@ void DoorLockServer::getCredentialStatusCommandHandler(chip::app::CommandHandler return; } + // Our response will need to include the index of the next occupied credential slot + // after credentialIndex, if there is one. + // + // We want to figure this out before we call emberAfPluginDoorLockGetCredential, because to do + // so we will also need to call emberAfPluginDoorLockGetCredential, and the + // EmberAfPluginDoorLockCredentialInfo we get might be pointing into some application-static + // buffers (for its credential data and whatnot). + DataModel::Nullable nextCredentialIndex; + { + uint16_t foundNextCredentialIndex; + if (findOccupiedCredentialSlot(commandPath.mEndpointId, credentialType, static_cast(credentialIndex + 1), + foundNextCredentialIndex)) + { + nextCredentialIndex.SetNonNull(foundNextCredentialIndex); + } + } + uint16_t maxNumberOfCredentials = 0; if (!credentialIndexValid(commandPath.mEndpointId, credentialType, credentialIndex, maxNumberOfCredentials)) { - sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, 0, nullptr, false); + sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, nextCredentialIndex, 0, nullptr, false); return; } @@ -896,8 +913,8 @@ void DoorLockServer::getCredentialStatusCommandHandler(chip::app::CommandHandler } } - sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, userIndexWithCredential, &credentialInfo, - credentialExists); + sendGetCredentialResponse(commandObj, commandPath, credentialType, credentialIndex, nextCredentialIndex, + userIndexWithCredential, &credentialInfo, credentialExists); } namespace { @@ -918,9 +935,12 @@ bool IsAliroCredentialType(CredentialTypeEnum credentialType) void DoorLockServer::sendGetCredentialResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, CredentialTypeEnum credentialType, uint16_t credentialIndex, - uint16_t userIndexWithCredential, + DataModel::Nullable nextCredentialIndex, uint16_t userIndexWithCredential, EmberAfPluginDoorLockCredentialInfo * credentialInfo, bool credentialExists) { + // Important: We have to make sure nothing in this function calls + // emberAfPluginDoorLockGetCredential, because that might stomp on the data + // pointed to by credentialInfo. Commands::GetCredentialStatusResponse::Type response{ .credentialExists = credentialExists }; if (credentialExists && !(nullptr == credentialInfo)) { @@ -949,19 +969,14 @@ void DoorLockServer::sendGetCredentialResponse(chip::app::CommandHandler * comma response.credentialData.Emplace(NullNullable); } } - uint16_t nextCredentialIndex = 0; - if (findOccupiedCredentialSlot(commandPath.mEndpointId, credentialType, static_cast(credentialIndex + 1), - nextCredentialIndex)) - { - response.nextCredentialIndex.SetNonNull(nextCredentialIndex); - } + response.nextCredentialIndex = nextCredentialIndex; commandObj->AddResponse(commandPath, response); ChipLogProgress(Zcl, "[GetCredentialStatus] Prepared credential status " "[endpointId=%d,credentialType=%u,credentialIndex=%d,userIndex=%d,nextCredentialIndex=%d]", commandPath.mEndpointId, to_underlying(credentialType), credentialIndex, userIndexWithCredential, - nextCredentialIndex); + nextCredentialIndex.ValueOr(0)); } void DoorLockServer::clearCredentialCommandHandler( diff --git a/src/app/clusters/door-lock-server/door-lock-server.h b/src/app/clusters/door-lock-server/door-lock-server.h index 81b18c60e687e8..6b888ce26943cb 100644 --- a/src/app/clusters/door-lock-server/door-lock-server.h +++ b/src/app/clusters/door-lock-server/door-lock-server.h @@ -449,7 +449,8 @@ class DoorLockServer : public chip::app::AttributeAccessInterface uint16_t credentialIndex); void sendGetCredentialResponse(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath, - CredentialTypeEnum credentialType, uint16_t credentialIndex, uint16_t userIndexWithCredential, + CredentialTypeEnum credentialType, uint16_t credentialIndex, + chip::app::DataModel::Nullable nextCredentialIndex, uint16_t userIndexWithCredential, EmberAfPluginDoorLockCredentialInfo * credentialInfo, bool credentialExists); void clearCredentialCommandHandler(chip::app::CommandHandler * commandObj, const chip::app::ConcreteCommandPath & commandPath,