From 1096607ff5297e82b36abe03751a8b7b7d4314e8 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Tue, 7 Mar 2023 23:36:46 -0500 Subject: [PATCH] Correctly return RESOURCE_EXHAUSTED when too many bindings are written. (#25549) Fixes https://github.com/project-chip/connectedhomeip/issues/25538 --- src/app/clusters/bindings/bindings.cpp | 35 ++++-- src/app/clusters/bindings/bindings.h | 2 +- src/app/tests/suites/TestBinding.yaml | 21 ++++ .../chip-tool/zap-generated/test/Commands.h | 108 +++++++++++++++++- .../zap-generated/test/Commands.h | 103 ++++++++++++++++- 5 files changed, 255 insertions(+), 14 deletions(-) diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp index 0b48cee6f6c4a4..22c4fba0be6920 100644 --- a/src/app/clusters/bindings/bindings.cpp +++ b/src/app/clusters/bindings/bindings.cpp @@ -30,6 +30,8 @@ #include #include #include +#include + using namespace chip; using namespace chip::app; using namespace chip::app::Clusters; @@ -117,7 +119,7 @@ CHIP_ERROR CheckValidBindingList(const EndpointId localEndpoint, const Decodable return CHIP_NO_ERROR; } -void CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint) +CHIP_ERROR CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint) { EmberBindingTableEntry bindingEntry; @@ -131,7 +133,7 @@ void CreateBindingEntry(const TargetStructType & entry, EndpointId localEndpoint entry.cluster); } - AddBindingEntry(bindingEntry); + return AddBindingEntry(bindingEntry); } CHIP_ERROR BindingTableAccess::Read(const ConcreteReadAttributePath & path, AttributeValueEncoder & encoder) @@ -225,10 +227,11 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath } // Add new entries - auto iter = newBindingList.begin(); - while (iter.Next()) + auto iter = newBindingList.begin(); + CHIP_ERROR err = CHIP_NO_ERROR; + while (iter.Next() && err == CHIP_NO_ERROR) { - CreateBindingEntry(iter.GetValue(), path.mEndpointId); + err = CreateBindingEntry(iter.GetValue(), path.mEndpointId); } // If this was not caused by a list operation, OnListWriteEnd is not going to be triggered @@ -238,7 +241,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath // Notify binding table has changed LogErrorOnFailure(NotifyBindingsChanged()); } - return CHIP_NO_ERROR; + return err; } if (path.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem) { @@ -248,8 +251,7 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath { return CHIP_IM_GLOBAL_STATUS(ConstraintError); } - CreateBindingEntry(target, path.mEndpointId); - return CHIP_NO_ERROR; + return CreateBindingEntry(target, path.mEndpointId); } return CHIP_IM_GLOBAL_STATUS(UnsupportedWrite); } @@ -269,11 +271,22 @@ void MatterBindingPluginServerInitCallback() registerAttributeAccessOverride(&gAttrAccess); } -void AddBindingEntry(const EmberBindingTableEntry & entry) +CHIP_ERROR AddBindingEntry(const EmberBindingTableEntry & entry) { + CHIP_ERROR err = BindingTable::GetInstance().Add(entry); + if (err == CHIP_ERROR_NO_MEMORY) + { + return CHIP_IM_GLOBAL_STATUS(ResourceExhausted); + } + + if (err != CHIP_NO_ERROR) + { + return err; + } + if (entry.type == EMBER_UNICAST_BINDING) { - CHIP_ERROR err = BindingManager::GetInstance().UnicastBindingCreated(entry.fabricIndex, entry.nodeId); + err = BindingManager::GetInstance().UnicastBindingCreated(entry.fabricIndex, entry.nodeId); if (err != CHIP_NO_ERROR) { // Unicast connection failure can happen if peer is offline. We'll retry connection on-demand. @@ -283,5 +296,5 @@ void AddBindingEntry(const EmberBindingTableEntry & entry) } } - BindingTable::GetInstance().Add(entry); + return CHIP_NO_ERROR; } diff --git a/src/app/clusters/bindings/bindings.h b/src/app/clusters/bindings/bindings.h index 3d6d773cdbcd8c..b77f7e99b2cedf 100644 --- a/src/app/clusters/bindings/bindings.h +++ b/src/app/clusters/bindings/bindings.h @@ -28,4 +28,4 @@ * * @param entry binding to add */ -void AddBindingEntry(const EmberBindingTableEntry & entry); +CHIP_ERROR AddBindingEntry(const EmberBindingTableEntry & entry); diff --git a/src/app/tests/suites/TestBinding.yaml b/src/app/tests/suites/TestBinding.yaml index 6ca70ad3d9c330..15f25cbb5f2365 100644 --- a/src/app/tests/suites/TestBinding.yaml +++ b/src/app/tests/suites/TestBinding.yaml @@ -104,3 +104,24 @@ tests: { FabricIndex: 1, Node: 1, Endpoint: 1, Cluster: 6 }, { FabricIndex: 1, Node: 2, Endpoint: 1 }, ] + + - label: "Write over-long binding table on endpoint 1" + command: "writeAttribute" + attribute: "Binding" + arguments: + value: + [ + { FabricIndex: 0, Node: 1, Endpoint: 1, Cluster: 6 }, + { FabricIndex: 0, Node: 2, Endpoint: 2, Cluster: 6 }, + { FabricIndex: 0, Node: 3, Endpoint: 3, Cluster: 6 }, + { FabricIndex: 0, Node: 4, Endpoint: 4, Cluster: 6 }, + { FabricIndex: 0, Node: 5, Endpoint: 5, Cluster: 6 }, + { FabricIndex: 0, Node: 6, Endpoint: 6, Cluster: 6 }, + { FabricIndex: 0, Node: 7, Endpoint: 7, Cluster: 6 }, + { FabricIndex: 0, Node: 8, Endpoint: 8, Cluster: 6 }, + { FabricIndex: 0, Node: 9, Endpoint: 9, Cluster: 6 }, + { FabricIndex: 0, Node: 10, Endpoint: 10, Cluster: 6 }, + { FabricIndex: 0, Node: 11, Endpoint: 11, Cluster: 6 }, + ] + response: + error: RESOURCE_EXHAUSTED diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index dd43f9aaafa5fe..c8b5d75ba7ac0e 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -70473,7 +70473,7 @@ class TestSystemCommandsSuite : public TestCommand class TestBindingSuite : public TestCommand { public: - TestBindingSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("TestBinding", 9, credsIssuerConfig) + TestBindingSuite(CredentialIssuerCommands * credsIssuerConfig) : TestCommand("TestBinding", 10, credsIssuerConfig) { AddArgument("nodeId", 0, UINT64_MAX, &mNodeId); AddArgument("cluster", &mCluster); @@ -70608,6 +70608,9 @@ class TestBindingSuite : public TestCommand } } break; + case 9: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_RESOURCE_EXHAUSTED)); + break; default: LogErrorOnFailure(ContinueOnChipMainThread(CHIP_ERROR_INVALID_ARGUMENT)); } @@ -70740,6 +70743,109 @@ class TestBindingSuite : public TestCommand return ReadAttribute(kIdentityAlpha, GetEndpoint(1), Binding::Id, Binding::Attributes::Binding::Id, true, chip::NullOptional); } + case 9: { + LogStep(9, "Write over-long binding table on endpoint 1"); + ListFreer listFreer; + chip::app::DataModel::List value; + + { + auto * listHolder_0 = new ListHolder(11); + listFreer.add(listHolder_0); + + listHolder_0->mList[0].node.Emplace(); + listHolder_0->mList[0].node.Value() = 1ULL; + listHolder_0->mList[0].endpoint.Emplace(); + listHolder_0->mList[0].endpoint.Value() = 1U; + listHolder_0->mList[0].cluster.Emplace(); + listHolder_0->mList[0].cluster.Value() = 6UL; + listHolder_0->mList[0].fabricIndex = 0U; + + listHolder_0->mList[1].node.Emplace(); + listHolder_0->mList[1].node.Value() = 2ULL; + listHolder_0->mList[1].endpoint.Emplace(); + listHolder_0->mList[1].endpoint.Value() = 2U; + listHolder_0->mList[1].cluster.Emplace(); + listHolder_0->mList[1].cluster.Value() = 6UL; + listHolder_0->mList[1].fabricIndex = 0U; + + listHolder_0->mList[2].node.Emplace(); + listHolder_0->mList[2].node.Value() = 3ULL; + listHolder_0->mList[2].endpoint.Emplace(); + listHolder_0->mList[2].endpoint.Value() = 3U; + listHolder_0->mList[2].cluster.Emplace(); + listHolder_0->mList[2].cluster.Value() = 6UL; + listHolder_0->mList[2].fabricIndex = 0U; + + listHolder_0->mList[3].node.Emplace(); + listHolder_0->mList[3].node.Value() = 4ULL; + listHolder_0->mList[3].endpoint.Emplace(); + listHolder_0->mList[3].endpoint.Value() = 4U; + listHolder_0->mList[3].cluster.Emplace(); + listHolder_0->mList[3].cluster.Value() = 6UL; + listHolder_0->mList[3].fabricIndex = 0U; + + listHolder_0->mList[4].node.Emplace(); + listHolder_0->mList[4].node.Value() = 5ULL; + listHolder_0->mList[4].endpoint.Emplace(); + listHolder_0->mList[4].endpoint.Value() = 5U; + listHolder_0->mList[4].cluster.Emplace(); + listHolder_0->mList[4].cluster.Value() = 6UL; + listHolder_0->mList[4].fabricIndex = 0U; + + listHolder_0->mList[5].node.Emplace(); + listHolder_0->mList[5].node.Value() = 6ULL; + listHolder_0->mList[5].endpoint.Emplace(); + listHolder_0->mList[5].endpoint.Value() = 6U; + listHolder_0->mList[5].cluster.Emplace(); + listHolder_0->mList[5].cluster.Value() = 6UL; + listHolder_0->mList[5].fabricIndex = 0U; + + listHolder_0->mList[6].node.Emplace(); + listHolder_0->mList[6].node.Value() = 7ULL; + listHolder_0->mList[6].endpoint.Emplace(); + listHolder_0->mList[6].endpoint.Value() = 7U; + listHolder_0->mList[6].cluster.Emplace(); + listHolder_0->mList[6].cluster.Value() = 6UL; + listHolder_0->mList[6].fabricIndex = 0U; + + listHolder_0->mList[7].node.Emplace(); + listHolder_0->mList[7].node.Value() = 8ULL; + listHolder_0->mList[7].endpoint.Emplace(); + listHolder_0->mList[7].endpoint.Value() = 8U; + listHolder_0->mList[7].cluster.Emplace(); + listHolder_0->mList[7].cluster.Value() = 6UL; + listHolder_0->mList[7].fabricIndex = 0U; + + listHolder_0->mList[8].node.Emplace(); + listHolder_0->mList[8].node.Value() = 9ULL; + listHolder_0->mList[8].endpoint.Emplace(); + listHolder_0->mList[8].endpoint.Value() = 9U; + listHolder_0->mList[8].cluster.Emplace(); + listHolder_0->mList[8].cluster.Value() = 6UL; + listHolder_0->mList[8].fabricIndex = 0U; + + listHolder_0->mList[9].node.Emplace(); + listHolder_0->mList[9].node.Value() = 10ULL; + listHolder_0->mList[9].endpoint.Emplace(); + listHolder_0->mList[9].endpoint.Value() = 10U; + listHolder_0->mList[9].cluster.Emplace(); + listHolder_0->mList[9].cluster.Value() = 6UL; + listHolder_0->mList[9].fabricIndex = 0U; + + listHolder_0->mList[10].node.Emplace(); + listHolder_0->mList[10].node.Value() = 11ULL; + listHolder_0->mList[10].endpoint.Emplace(); + listHolder_0->mList[10].endpoint.Value() = 11U; + listHolder_0->mList[10].cluster.Emplace(); + listHolder_0->mList[10].cluster.Value() = 6UL; + listHolder_0->mList[10].fabricIndex = 0U; + + value = + chip::app::DataModel::List(listHolder_0->mList, 11); + } + return WriteAttribute(kIdentityAlpha, GetEndpoint(1), Binding::Id, Binding::Attributes::Binding::Id, value, + chip::NullOptional, chip::NullOptional); + } } return CHIP_NO_ERROR; } diff --git a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h index 211cf0f0c7a45b..36318231e2cb0d 100644 --- a/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h +++ b/zzz_generated/darwin-framework-tool/zap-generated/test/Commands.h @@ -102449,6 +102449,10 @@ class TestBinding : public TestCommandBridge { ChipLogProgress(chipTool, " ***** Test Step 8 : Verify endpoint 1 not changed\n"); err = TestVerifyEndpoint1NotChanged_8(); break; + case 9: + ChipLogProgress(chipTool, " ***** Test Step 9 : Write over-long binding table on endpoint 1\n"); + err = TestWriteOverLongBindingTableOnEndpoint1_9(); + break; } if (CHIP_NO_ERROR != err) { @@ -102487,6 +102491,9 @@ class TestBinding : public TestCommandBridge { case 8: VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), 0)); break; + case 9: + VerifyOrReturn(CheckValue("status", chip::to_underlying(status.mStatus), EMBER_ZCL_STATUS_RESOURCE_EXHAUSTED)); + break; } // Go on to the next test. @@ -102500,7 +102507,7 @@ class TestBinding : public TestCommandBridge { private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 9; + const uint16_t mTestCount = 10; chip::Optional mNodeId; chip::Optional mCluster; @@ -102790,6 +102797,100 @@ class TestBinding : public TestCommandBridge { return CHIP_NO_ERROR; } + + CHIP_ERROR TestWriteOverLongBindingTableOnEndpoint1_9() + { + + MTRBaseDevice * device = GetDevice("alpha"); + __auto_type * cluster = [[MTRBaseClusterBinding alloc] initWithDevice:device endpointID:@(1) queue:mCallbackQueue]; + VerifyOrReturnError(cluster != nil, CHIP_ERROR_INCORRECT_STATE); + + id bindingArgument; + { + NSMutableArray * temp_0 = [[NSMutableArray alloc] init]; + temp_0[0] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[0]).node = [NSNumber numberWithUnsignedLongLong:1ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[0]).endpoint = [NSNumber numberWithUnsignedShort:1U]; + ((MTRBindingClusterTargetStruct *) temp_0[0]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[0]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[1] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[1]).node = [NSNumber numberWithUnsignedLongLong:2ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[1]).endpoint = [NSNumber numberWithUnsignedShort:2U]; + ((MTRBindingClusterTargetStruct *) temp_0[1]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[1]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[2] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[2]).node = [NSNumber numberWithUnsignedLongLong:3ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[2]).endpoint = [NSNumber numberWithUnsignedShort:3U]; + ((MTRBindingClusterTargetStruct *) temp_0[2]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[2]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[3] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[3]).node = [NSNumber numberWithUnsignedLongLong:4ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[3]).endpoint = [NSNumber numberWithUnsignedShort:4U]; + ((MTRBindingClusterTargetStruct *) temp_0[3]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[3]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[4] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[4]).node = [NSNumber numberWithUnsignedLongLong:5ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[4]).endpoint = [NSNumber numberWithUnsignedShort:5U]; + ((MTRBindingClusterTargetStruct *) temp_0[4]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[4]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[5] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[5]).node = [NSNumber numberWithUnsignedLongLong:6ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[5]).endpoint = [NSNumber numberWithUnsignedShort:6U]; + ((MTRBindingClusterTargetStruct *) temp_0[5]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[5]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[6] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[6]).node = [NSNumber numberWithUnsignedLongLong:7ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[6]).endpoint = [NSNumber numberWithUnsignedShort:7U]; + ((MTRBindingClusterTargetStruct *) temp_0[6]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[6]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[7] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[7]).node = [NSNumber numberWithUnsignedLongLong:8ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[7]).endpoint = [NSNumber numberWithUnsignedShort:8U]; + ((MTRBindingClusterTargetStruct *) temp_0[7]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[7]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[8] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[8]).node = [NSNumber numberWithUnsignedLongLong:9ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[8]).endpoint = [NSNumber numberWithUnsignedShort:9U]; + ((MTRBindingClusterTargetStruct *) temp_0[8]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[8]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[9] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[9]).node = [NSNumber numberWithUnsignedLongLong:10ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[9]).endpoint = [NSNumber numberWithUnsignedShort:10U]; + ((MTRBindingClusterTargetStruct *) temp_0[9]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[9]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + temp_0[10] = [[MTRBindingClusterTargetStruct alloc] init]; + ((MTRBindingClusterTargetStruct *) temp_0[10]).node = [NSNumber numberWithUnsignedLongLong:11ULL]; + ((MTRBindingClusterTargetStruct *) temp_0[10]).endpoint = [NSNumber numberWithUnsignedShort:11U]; + ((MTRBindingClusterTargetStruct *) temp_0[10]).cluster = [NSNumber numberWithUnsignedInt:6UL]; + ((MTRBindingClusterTargetStruct *) temp_0[10]).fabricIndex = [NSNumber numberWithUnsignedChar:0U]; + + bindingArgument = temp_0; + } + [cluster + writeAttributeBindingWithValue:bindingArgument + completion:^(NSError * _Nullable err) { + NSLog(@"Write over-long binding table on endpoint 1 Error: %@", err); + + VerifyOrReturn(CheckValue("status", + err ? ([err.domain isEqualToString:MTRInteractionErrorDomain] ? err.code + : EMBER_ZCL_STATUS_FAILURE) + : 0, + EMBER_ZCL_STATUS_RESOURCE_EXHAUSTED)); + NextTest(); + }]; + + return CHIP_NO_ERROR; + } }; class TestUserLabelCluster : public TestCommandBridge {