On 11/07/2021 22:51, Ranier Vilela wrote:
Hi,
While analyzing a possible use of an uninitialized variable, I checked that
*_bt_restore_page* can lead to memory corruption,
by not checking the maximum limit of array items which is
MaxIndexTuplesPerPage.
+ /* Protect against corrupted recovery file */
+ nitems = (len / sizeof(IndexTupleData));
+ if (nitems < 0 || nitems > MaxIndexTuplesPerPage)
+ elog(PANIC, "_bt_restore_page: cannot restore %d items to
page", nitems);
+
That's not right. You don't get the number of items by dividing like
that. 'len' includes the tuple data as well, not just the IndexTupleData
header.
@@ -73,12 +79,9 @@ _bt_restore_page(Page page, char *from, int len)
nitems = i;
for (i = nitems - 1; i >= 0; i--)
- {
if (PageAddItem(page, items[i], itemsizes[i], nitems - i,
false, false) ==
InvalidOffsetNumber)
elog(PANIC, "_bt_restore_page: cannot add item to
page");
- from += itemsz;
- }
}
I agree with this change (except that I would leave the braces in
place). The 'from' that's calculated here is plain wrong; oversight in
commit 7e30c186da. Fortunately it's not used, so it can just be removed.
- Heikki
