From ed25adc3dbc41f8e8b54b46ab327ccc8f8177e2f Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Sat, 9 Dec 2023 00:35:01 +0000 Subject: [PATCH 1/3] Fix #109024: Off-by-1 in rna_access for non-array props without raw access The `a + itemlen > in.len` check was off-by-1 whenever accessing a non-array property without raw access. This was because `itemlen` was actually the array length of the property, which is `0` for non-array properties. Given an array which was too short, this would cause the slower loop to overrun the end of the array by one item, causing a crash on a debug build with `Fatal Python error: _PyMem_DebugRawFree: bad trailing pad byte`. The semantics of the `itemlen` and `arraylen` variables have now been swapped to be less confusing. So `arraylen` is now the array length of the property and `itemlen` is now the item length of the property. The `arraylen` variable has been given the same scope as `itemlen` so that it can be used in the slower loop when raw access is not available. --- source/blender/makesrna/intern/rna_access.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc index a70a0fc5913..aaeb18c5d09 100644 --- a/source/blender/makesrna/intern/rna_access.cc +++ b/source/blender/makesrna/intern/rna_access.cc @@ -4551,7 +4551,7 @@ static int rna_raw_access(ReportList *reports, PropertyRNA *itemprop, *iprop; PropertyType itemtype = PropertyType(0); RawArray in; - int itemlen = 0; + int itemlen = 0, arraylen = 0; /* initialize in array, stride assumed 0 in following code */ in.array = inarray; @@ -4578,7 +4578,8 @@ static int rna_raw_access(ReportList *reports, } /* check item array */ - itemlen = RNA_property_array_length(&itemptr_base, itemprop); + arraylen = RNA_property_array_length(&itemptr_base, itemprop); + itemlen = (arraylen == 0) ? 1 : arraylen; /* dynamic array? need to get length per item */ if (itemprop->getlength) { @@ -4586,8 +4587,7 @@ static int rna_raw_access(ReportList *reports, } /* try to access as raw array */ else if (RNA_property_collection_raw_array(ptr, prop, itemprop, set, &out)) { - int arraylen = (itemlen == 0) ? 1 : itemlen; - if (in.len != arraylen * out.len) { + if (in.len != itemlen * out.len) { BKE_reportf(reports, RPT_ERROR, "Array length mismatch (expected %d, got %d)", @@ -4650,7 +4650,8 @@ static int rna_raw_access(ReportList *reports, iprop = RNA_struct_find_property(&itemptr, propname); if (iprop) { - itemlen = rna_property_array_length_all_dimensions(&itemptr, iprop); + arraylen = rna_property_array_length_all_dimensions(&itemptr, iprop); + itemlen = (arraylen == 0) ? 1 : arraylen; itemtype = RNA_property_type(iprop); } else { @@ -4675,7 +4676,7 @@ static int rna_raw_access(ReportList *reports, break; } - if (itemlen == 0) { + if (arraylen == 0) { /* handle conversions */ if (set) { switch (itemtype) { -- 2.30.2 From e78c8fa210c2df2a51385eed0315ae54f13d0ce1 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Sun, 10 Dec 2023 02:47:01 +0000 Subject: [PATCH 2/3] Missed variable changes in previous commit --- source/blender/makesrna/intern/rna_access.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc index aaeb18c5d09..4955a5e055f 100644 --- a/source/blender/makesrna/intern/rna_access.cc +++ b/source/blender/makesrna/intern/rna_access.cc @@ -4591,7 +4591,7 @@ static int rna_raw_access(ReportList *reports, BKE_reportf(reports, RPT_ERROR, "Array length mismatch (expected %d, got %d)", - out.len * arraylen, + out.len * itemlen, in.len); return 0; } @@ -4602,7 +4602,7 @@ static int rna_raw_access(ReportList *reports, void *outp = out.array; int a, size; - size = RNA_raw_type_sizeof(out.type) * arraylen; + size = RNA_raw_type_sizeof(out.type) * itemlen; for (a = 0; a < out.len; a++) { if (set) { -- 2.30.2 From 55430e9666648d68e65a1c680ed455fd001f8e06 Mon Sep 17 00:00:00 2001 From: Thomas Barlow Date: Wed, 13 Dec 2023 03:38:08 +0000 Subject: [PATCH 3/3] Update the recently added enum array asserts to use `arraylen` This patch swaps the semantics of `arraylen` and `itemlen` and the recently added asserts were using the old `itemlen`. --- source/blender/makesrna/intern/rna_access.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/blender/makesrna/intern/rna_access.cc b/source/blender/makesrna/intern/rna_access.cc index 646cd59ce51..06b1be0842b 100644 --- a/source/blender/makesrna/intern/rna_access.cc +++ b/source/blender/makesrna/intern/rna_access.cc @@ -4622,7 +4622,7 @@ static int rna_raw_access(ReportList *reports, /* Could also be faster with non-matching types, * for now we just do slower loop. */ } - BLI_assert_msg(itemlen == 0 || itemtype != PROP_ENUM, + BLI_assert_msg(arraylen == 0 || itemtype != PROP_ENUM, "Enum array properties should not exist"); } @@ -4668,7 +4668,7 @@ static int rna_raw_access(ReportList *reports, err = 1; break; } - BLI_assert_msg(itemlen == 0 || itemtype != PROP_ENUM, + BLI_assert_msg(arraylen == 0 || itemtype != PROP_ENUM, "Enum array properties should not exist"); } -- 2.30.2