Fix stale metadata with disabled encryption (#16887) #16888
Conversation
|
🟢 |
There was a problem hiding this comment.
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl_metadata.cpp:44
- The inline reinterpret_cast of 'PDisk->Format.ChunkKey' to 'ui64*' may violate strict aliasing rules and lead to undefined behavior; consider using a helper function or a safer conversion method to extract the magic value.
if (header->CheckHash((ui64*)(void*)&PDisk->Format.ChunkKey)) {
ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl_metadata.cpp:596
- The reinterpret_cast used to pass the magic value from 'Format.ChunkKey' is repeated here; consider abstracting this conversion into a helper function to improve safety and maintainability.
CreateMetadataPayload(write.Metadata, offset, payloadSize, Format.SectorSize, Cfg->EnableSectorEncryption, Format.ChunkKey, Meta.NextSequenceNumber, i, numSlotsRequired, (ui64*)(void*)&Format.ChunkKey);
ydb/core/blobstorage/pdisk/blobstorage_pdisk_impl_metadata.cpp:620
- The repeated pattern of reinterpret_cast to obtain a pointer to the magic value from 'fmt.DataKey' could be refactored into a well-defined helper for improved clarity and to avoid potential undefined behavior.
CreateMetadataPayload(write.Metadata, 0, write.Metadata.size(), DefaultSectorSize, true, fmt.DataKey, 0, 0, 1, (ui64*)(void*)&fmt.DataKey);
| auto *header = reinterpret_cast<TMetadataHeader*>(Buffer.GetDataMut()); | ||
| header->Encrypt(cypher); | ||
| if (!header->CheckHash()) { | ||
| if (!header->CheckHash((ui64*)(void*)&Format.DataKey)) { |
There was a problem hiding this comment.
Similar to the previous instance, using reinterpret_cast to pass 'Format.DataKey' as a magic pointer may lead to undefined behavior; a safer, more explicit conversion method is recommended.
| if (!header->CheckHash((ui64*)(void*)&Format.DataKey)) { | |
| ui64 dataKey; | |
| std::memcpy(&dataKey, &Format.DataKey, sizeof(ui64)); | |
| if (!header->CheckHash(&dataKey)) { |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ DetailsTest history | Ya make output | Test bloat
⚪ DetailsTest history | Ya make output | Test bloat | Test bloat
⚪ Test history | Ya make output | Test bloat | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
2 participants
Changelog entry
...
Changelog category
Description for reviewers
Metadata was unencrypted and format sector obliteration did not render it unreadable. Hash the key so that obliteration works once again.