From b4cfb0c8168995b55563ea3b20ff27577ae6698b Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 15 Sep 2024 23:07:17 +1000 Subject: [PATCH] BKE_lib: avoid potential read uninitialized memory in BKE_id_copy_* BKE_id_copy_in_lib took an argument `r_newid` which was was to initialize `new_id` in the functions body. While this may not have caused any user visible bugs, it's error prone, one caller even passed in an uninitialized pointer. - Rename `r_newid` to `new_id_p` since it's not a return argument. - Initialize the value from all callers. --- source/blender/blenkernel/BKE_lib_id.hh | 10 ++++----- source/blender/blenkernel/intern/lib_id.cc | 26 +++++++++++----------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/source/blender/blenkernel/BKE_lib_id.hh b/source/blender/blenkernel/BKE_lib_id.hh index e53f543f052..8e326b98458 100644 --- a/source/blender/blenkernel/BKE_lib_id.hh +++ b/source/blender/blenkernel/BKE_lib_id.hh @@ -218,7 +218,7 @@ enum { LIB_ID_COPY_NO_LIB_OVERRIDE, }; -void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **r_newid, int orig_flag); +void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **new_id_p, int orig_flag); /** * Same as #BKE_libblock_copy_ex, but allows copying data into a library, and not as local data * only. @@ -233,7 +233,7 @@ void BKE_libblock_copy_in_lib(Main *bmain, std::optional owner_library, const ID *id, const ID *new_owner_id, - ID **r_newid, + ID **new_id_p, int orig_flag); /** @@ -503,13 +503,13 @@ bool BKE_id_copy_is_allowed(const ID *id); * * \param bmain: Main database, may be NULL only if LIB_ID_CREATE_NO_MAIN is specified. * \param id: Source data-block. - * \param r_newid: Pointer to new (copied) ID pointer, may be NULL. + * \param new_id_p: Pointer to new (copied) ID pointer, may be NULL. * Used to allow copying into already allocated memory. * \param flag: Set of copy options, see `DNA_ID.h` enum for details * (leave to zero for default, full copy). * \return NULL when copying that ID type is not supported, the new copy otherwise. */ -ID *BKE_id_copy_ex(Main *bmain, const ID *id, ID **r_newid, int flag); +ID *BKE_id_copy_ex(Main *bmain, const ID *id, ID **new_id_p, int flag); /** * Enable coying non-local data into libraries. * @@ -525,7 +525,7 @@ struct ID *BKE_id_copy_in_lib(Main *bmain, std::optional owner_library, const ID *id, const ID *new_owner_id, - ID **r_newid, + ID **new_id_p, int flag); /** * Invoke the appropriate copy method for the block and return the new id as result. diff --git a/source/blender/blenkernel/intern/lib_id.cc b/source/blender/blenkernel/intern/lib_id.cc index e20cdaa8974..5e9c610839a 100644 --- a/source/blender/blenkernel/intern/lib_id.cc +++ b/source/blender/blenkernel/intern/lib_id.cc @@ -637,10 +637,10 @@ ID *BKE_id_copy_in_lib(Main *bmain, std::optional owner_library, const ID *id, const ID *new_owner_id, - ID **r_newid, + ID **new_id_p, const int flag) { - ID *newid = (r_newid != nullptr) ? *r_newid : nullptr; + ID *newid = (new_id_p != nullptr) ? *new_id_p : nullptr; BLI_assert_msg(newid || (flag & LIB_ID_CREATE_NO_ALLOCATE) == 0, "Copying with 'no allocate' behavior should always get a non-null new ID buffer"); @@ -713,16 +713,16 @@ ID *BKE_id_copy_in_lib(Main *bmain, newid->lib = owner_library ? *owner_library : id->lib; } - if (r_newid != nullptr) { - *r_newid = newid; + if (new_id_p != nullptr) { + *new_id_p = newid; } return newid; } -ID *BKE_id_copy_ex(Main *bmain, const ID *id, ID **r_newid, const int flag) +ID *BKE_id_copy_ex(Main *bmain, const ID *id, ID **new_id_p, const int flag) { - return BKE_id_copy_in_lib(bmain, std::nullopt, id, nullptr, r_newid, flag); + return BKE_id_copy_in_lib(bmain, std::nullopt, id, nullptr, new_id_p, flag); } ID *BKE_id_copy(Main *bmain, const ID *id) @@ -1459,10 +1459,10 @@ void BKE_libblock_copy_in_lib(Main *bmain, std::optional owner_library, const ID *id, const ID *new_owner_id, - ID **r_newid, + ID **new_id_p, const int orig_flag) { - ID *new_id = *r_newid; + ID *new_id = *new_id_p; int flag = orig_flag; const bool is_embedded_id = (id->flag & LIB_EMBEDDED_DATA) != 0; @@ -1484,7 +1484,7 @@ void BKE_libblock_copy_in_lib(Main *bmain, const int copy_idflag_mask = LIB_EMBEDDED_DATA; if ((flag & LIB_ID_CREATE_NO_ALLOCATE) != 0) { - /* r_newid already contains pointer to allocated memory. */ + /* `new_id_p` already contains pointer to allocated memory. */ /* TODO: do we want to memset(0) whole mem before filling it? */ STRNCPY(new_id->name, id->name); new_id->us = 0; @@ -1576,17 +1576,17 @@ void BKE_libblock_copy_in_lib(Main *bmain, DEG_id_type_tag(bmain, GS(new_id->name)); } - *r_newid = new_id; + *new_id_p = new_id; } -void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **r_newid, const int orig_flag) +void BKE_libblock_copy_ex(Main *bmain, const ID *id, ID **new_id_p, const int orig_flag) { - BKE_libblock_copy_in_lib(bmain, std::nullopt, id, nullptr, r_newid, orig_flag); + BKE_libblock_copy_in_lib(bmain, std::nullopt, id, nullptr, new_id_p, orig_flag); } void *BKE_libblock_copy(Main *bmain, const ID *id) { - ID *idn; + ID *idn = nullptr; BKE_libblock_copy_in_lib(bmain, std::nullopt, id, nullptr, &idn, 0);