From 004b9ea4c4bae4aa236048b23469c0374c5e72dc Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 13 Jun 2024 17:47:59 +0200 Subject: [PATCH] src: make sure that memcpy-ed structs in snapshot have no padding To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs. PR-URL: https://github.com/nodejs/node/pull/50983 Refs: https://github.com/nodejs/build/issues/3043 Reviewed-By: Daniel Lemire Reviewed-By: James M Snell --- src/aliased_buffer-inl.h | 2 +- src/aliased_buffer.h | 2 +- src/base_object_types.h | 7 +++++-- src/encoding_binding.h | 6 ++++++ src/node_file.h | 6 ++++++ src/node_process.h | 6 ++++++ src/node_snapshotable.cc | 12 ++++++------ src/node_snapshotable.h | 37 +++++++++++++++++++++++++++++++++---- src/node_v8.h | 7 +++++++ 9 files changed, 71 insertions(+), 14 deletions(-) diff --git a/src/aliased_buffer-inl.h b/src/aliased_buffer-inl.h index f04fb7c2667016..40e08dfe149667 100644 --- a/src/aliased_buffer-inl.h +++ b/src/aliased_buffer-inl.h @@ -8,7 +8,7 @@ namespace node { -typedef size_t AliasedBufferIndex; +typedef uint64_t AliasedBufferIndex; template AliasedBufferBase::AliasedBufferBase( diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index b847641f8faa15..c2ddd269fd179c 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -9,7 +9,7 @@ namespace node { -typedef size_t AliasedBufferIndex; +typedef uint64_t AliasedBufferIndex; /** * Do not use this class directly when creating instances of it - use the diff --git a/src/base_object_types.h b/src/base_object_types.h index 9cfe6a77f71708..391a88973171ab 100644 --- a/src/base_object_types.h +++ b/src/base_object_types.h @@ -42,10 +42,13 @@ namespace node { SERIALIZABLE_NON_BINDING_TYPES(V) #define V(TypeId, NativeType) k_##TypeId, -enum class BindingDataType : uint8_t { BINDING_TYPES(V) kBindingDataTypeCount }; +// To avoid padding, the enums are uint64_t. +enum class BindingDataType : uint64_t { + BINDING_TYPES(V) kBindingDataTypeCount +}; // Make sure that we put the bindings first so that we can also use the enums // for the bindings as index to the binding data store. -enum class EmbedderObjectType : uint8_t { +enum class EmbedderObjectType : uint64_t { BINDING_TYPES(V) SERIALIZABLE_NON_BINDING_TYPES(V) // We do not need to know about all the unserializable non-binding types for // now so we do not list them. diff --git a/src/encoding_binding.h b/src/encoding_binding.h index 2690cb74f8a05b..7b18db9c6a1108 100644 --- a/src/encoding_binding.h +++ b/src/encoding_binding.h @@ -18,6 +18,12 @@ class BindingData : public SnapshotableObject { AliasedBufferIndex encode_into_results_buffer; }; + // Make sure that there's no padding in the struct since we will memcpy + // them into the snapshot blob and they need to be reproducible. + static_assert(sizeof(InternalFieldInfo) == + sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex), + "InternalFieldInfo should have no padding"); + BindingData(Realm* realm, v8::Local obj, InternalFieldInfo* info = nullptr); diff --git a/src/node_file.h b/src/node_file.h index bdad1ae25f4892..a0144314b280c1 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -64,6 +64,12 @@ class BindingData : public SnapshotableObject { AliasedBufferIndex statfs_field_bigint_array; }; + // Make sure that there's no padding in the struct since we will memcpy + // them into the snapshot blob and they need to be reproducible. + static_assert(sizeof(InternalFieldInfo) == sizeof(InternalFieldInfoBase) + + sizeof(AliasedBufferIndex) * 4, + "InternalFieldInfo should have no padding"); + enum class FilePathIsFileReturnType { kIsFile = 0, kIsNotFile, diff --git a/src/node_process.h b/src/node_process.h index 142d0e63e18c46..607826398beeae 100644 --- a/src/node_process.h +++ b/src/node_process.h @@ -54,6 +54,12 @@ class BindingData : public SnapshotableObject { AliasedBufferIndex hrtime_buffer; }; + // Make sure that there's no padding in the struct since we will memcpy + // them into the snapshot blob and they need to be reproducible. + static_assert(sizeof(InternalFieldInfo) == + sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex), + "InternalFieldInfo should have no padding"); + static void AddMethods(v8::Isolate* isolate, v8::Local target); static void RegisterExternalReferences(ExternalReferenceRegistry* registry); diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index aefd479b83cd05..f665d9dad01feb 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -282,9 +282,9 @@ size_t SnapshotSerializer::Write(const PropInfo& data) { } // Layout of AsyncHooks::SerializeInfo -// [ 4/8 bytes ] snapshot index of async_ids_stack -// [ 4/8 bytes ] snapshot index of fields -// [ 4/8 bytes ] snapshot index of async_id_fields +// [ 8 bytes ] snapshot index of async_ids_stack +// [ 8 bytes ] snapshot index of fields +// [ 8 bytes ] snapshot index of async_id_fields // [ 4/8 bytes ] snapshot index of js_execution_async_resources // [ 4/8 bytes ] length of native_execution_async_resources // [ ... ] snapshot indices of each element in @@ -387,9 +387,9 @@ size_t SnapshotSerializer::Write(const ImmediateInfo::SerializeInfo& data) { } // Layout of PerformanceState::SerializeInfo -// [ 4/8 bytes ] snapshot index of root -// [ 4/8 bytes ] snapshot index of milestones -// [ 4/8 bytes ] snapshot index of observers +// [ 8 bytes ] snapshot index of root +// [ 8 bytes ] snapshot index of milestones +// [ 8 bytes ] snapshot index of observers template <> performance::PerformanceState::SerializeInfo SnapshotDeserializer::Read() { Debug("Read()\n"); diff --git a/src/node_snapshotable.h b/src/node_snapshotable.h index 600e56c481be90..51a9499da5f2d7 100644 --- a/src/node_snapshotable.h +++ b/src/node_snapshotable.h @@ -4,6 +4,8 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS +#include // For static_assert +#include // For offsetof #include "aliased_buffer.h" #include "base_object.h" #include "util.h" @@ -33,13 +35,13 @@ bool WithoutCodeCache(const SnapshotConfig& config); // and pass it into the V8 callback as the payload of StartupData. // The memory chunk looks like this: // -// [ type ] - EmbedderObjectType (a uint8_t) -// [ length ] - a size_t +// [ type ] - EmbedderObjectType (a uint64_t) +// [ length ] - a uint64_t // [ ... ] - custom bytes of size |length - header size| struct InternalFieldInfoBase { public: EmbedderObjectType type; - size_t length; + uint64_t length; template static T* New(EmbedderObjectType type) { @@ -71,14 +73,35 @@ struct InternalFieldInfoBase { InternalFieldInfoBase() = default; }; +// Make sure that there's no padding in the struct since we will memcpy +// them into the snapshot blob and they need to be reproducible. +static_assert(offsetof(InternalFieldInfoBase, type) == 0, + "InternalFieldInfoBase::type should start from offset 0"); +static_assert(offsetof(InternalFieldInfoBase, length) == + sizeof(EmbedderObjectType), + "InternalFieldInfoBase::type should have no padding"); + struct EmbedderTypeInfo { - enum class MemoryMode : uint8_t { kBaseObject, kCppGC }; + // To avoid padding, the enum is uint64_t. + enum class MemoryMode : uint64_t { kBaseObject = 0, kCppGC }; EmbedderTypeInfo(EmbedderObjectType t, MemoryMode m) : type(t), mode(m) {} EmbedderTypeInfo() = default; + EmbedderObjectType type; MemoryMode mode; }; +// Make sure that there's no padding in the struct since we will memcpy +// them into the snapshot blob and they need to be reproducible. +static_assert(offsetof(EmbedderTypeInfo, type) == 0, + "EmbedderTypeInfo::type should start from offset 0"); +static_assert(offsetof(EmbedderTypeInfo, mode) == sizeof(EmbedderObjectType), + "EmbedderTypeInfo::type should have no padding"); +static_assert(sizeof(EmbedderTypeInfo) == + sizeof(EmbedderObjectType) + + sizeof(EmbedderTypeInfo::MemoryMode), + "EmbedderTypeInfo::mode should have no padding"); + // An interface for snapshotable native objects to inherit from. // Use the SERIALIZABLE_OBJECT_METHODS() macro in the class to define // the following methods to implement: @@ -150,6 +173,12 @@ class BindingData : public SnapshotableObject { AliasedBufferIndex is_building_snapshot_buffer; }; + // Make sure that there's no padding in the struct since we will memcpy + // them into the snapshot blob and they need to be reproducible. + static_assert(sizeof(InternalFieldInfo) == + sizeof(InternalFieldInfoBase) + sizeof(AliasedBufferIndex), + "InternalFieldInfo should have no padding"); + BindingData(Realm* realm, v8::Local obj, InternalFieldInfo* info = nullptr); diff --git a/src/node_v8.h b/src/node_v8.h index d3797432a24db7..844addd6a93b1b 100644 --- a/src/node_v8.h +++ b/src/node_v8.h @@ -23,6 +23,13 @@ class BindingData : public SnapshotableObject { AliasedBufferIndex heap_space_statistics_buffer; AliasedBufferIndex heap_code_statistics_buffer; }; + + // Make sure that there's no padding in the struct since we will memcpy + // them into the snapshot blob and they need to be reproducible. + static_assert(sizeof(InternalFieldInfo) == sizeof(InternalFieldInfoBase) + + sizeof(AliasedBufferIndex) * 3, + "InternalFieldInfo should have no padding"); + BindingData(Realm* realm, v8::Local obj, InternalFieldInfo* info = nullptr);