Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Commissioning time sync: copy parameters to autocommissioner #29908

Merged
merged 3 commits into from
Oct 23, 2023
Merged

Commissioning time sync: copy parameters to autocommissioner #29908

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5e0ea5
Commissioning time sync: copy parameters to autocommissioner
cecille Oct 22, 2023
6fc80bc
Restyled by clang-format
restyled-commits Oct 23, 2023
f414b09
use a min function rather than being crazy
cecille Oct 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/controller/AutoCommissioner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
IsUnsafeSpan(params.GetIcac(), mParams.GetIcac()) || IsUnsafeSpan(params.GetIpk(), mParams.GetIpk()) ||
IsUnsafeSpan(params.GetAttestationElements(), mParams.GetAttestationElements()) ||
IsUnsafeSpan(params.GetAttestationSignature(), mParams.GetAttestationSignature()) ||
IsUnsafeSpan(params.GetPAI(), mParams.GetPAI()) || IsUnsafeSpan(params.GetDAC(), mParams.GetDAC()));
IsUnsafeSpan(params.GetPAI(), mParams.GetPAI()) || IsUnsafeSpan(params.GetDAC(), mParams.GetDAC()) ||
IsUnsafeSpan(params.GetTimeZone(), mParams.GetTimeZone()) ||
IsUnsafeSpan(params.GetDSTOffsets(), mParams.GetDSTOffsets()));

mParams = params;

Expand Down Expand Up @@ -174,6 +176,33 @@ CHIP_ERROR AutoCommissioner::SetCommissioningParameters(const CommissioningParam
}
mParams.SetCSRNonce(ByteSpan(mCSRNonce, sizeof(mCSRNonce)));

if (params.GetDSTOffsets().HasValue())
{
ChipLogProgress(Controller, "Setting DST offsets from parameters");
size_t size = std::min(params.GetDSTOffsets().Value().size(), kMaxSupportedDstStructs);
for (size_t i = 0; i < size; ++i)
{
mDstOffsetsBuf[i] = params.GetDSTOffsets().Value()[i];
}
auto list = app::DataModel::List<app::Clusters::TimeSynchronization::Structs::DSTOffsetStruct::Type>(mDstOffsetsBuf, size);
mParams.SetDSTOffsets(list);
}
if (params.GetTimeZone().HasValue())
{
ChipLogProgress(Controller, "Setting Time Zone from parameters");
size_t size = std::min(params.GetTimeZone().Value().size(), kMaxSupportedTimeZones);
for (size_t i = 0; i < size; ++i)
{
mTimeZoneBuf[i] = params.GetTimeZone().Value()[i];
if (mTimeZoneBuf[i].name.HasValue())
{
auto span = MutableCharSpan(mTimeZoneNames[i], kMaxTimeZoneNameLen);
CopyCharSpanToMutableCharSpan(mTimeZoneBuf[i].name.Value(), span);
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If this fails, won't this leave garbage data in mTimeZoneNames[i]? And leave span as garbage pointing to possibly un-initialized memory too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why would that ever fail? It's a statically allocated buffer that's been size checked.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think mTimeZoneBuf[i].name.Value() has been size-checked...

mTimeZoneBuf[i].name.SetValue(span);
}
}
}

return CHIP_NO_ERROR;
}

Expand Down
11 changes: 11 additions & 0 deletions src/controller/AutoCommissioner.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ class AutoCommissioner : public CommissioningDelegate
uint8_t mThreadOperationalDataset[CommissioningParameters::kMaxThreadDatasetLen];
char mCountryCode[CommissioningParameters::kMaxCountryCodeLen];

// Time zone is statically allocated because it is max 2 and not trivially destructible
static constexpr size_t kMaxSupportedTimeZones = 2;
app::Clusters::TimeSynchronization::Structs::TimeZoneStruct::Type mTimeZoneBuf[kMaxSupportedTimeZones];
static constexpr size_t kMaxTimeZoneNameLen = 64;
char mTimeZoneNames[kMaxTimeZoneNameLen][kMaxSupportedTimeZones];

// DSTOffsetStructs are similarly not trivially destructible. They don't have a defined size, but we're
// going to do static allocation of the buffers anyway until we replace chip::Optional with std::optional.
static constexpr size_t kMaxSupportedDstStructs = 10;
app::Clusters::TimeSynchronization::Structs::DSTOffsetStruct::Type mDstOffsetsBuf[kMaxSupportedDstStructs];

bool mNeedsNetworkSetup = false;
ReadCommissioningInfo mDeviceCommissioningInfo;
bool mNeedsDST = false;
Expand Down
2 changes: 2 additions & 0 deletions src/controller/CommissioningDelegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,8 @@ class CommissioningParameters
mAttestationSignature.ClearValue();
mPAI.ClearValue();
mDAC.ClearValue();
mTimeZone.ClearValue();
mDSTOffsets.ClearValue();
}

private:
Expand Down
Loading