From 87bbc058228bba79c4640ba810248e6c3780a012 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 6 Mar 2024 17:44:19 +1300 Subject: [PATCH v2] Fix potential stack overflow in incremental basebackup. Since the user can set to RELSEG_SIZE to a high number at compile time, it seems unwise to use it to size an array on the stack. But on closer inspection, we can skip that intermediate array and just write directly into the output array. --- src/backend/backup/basebackup_incremental.c | 27 ++++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/backend/backup/basebackup_incremental.c b/src/backend/backup/basebackup_incremental.c index 131598bade..abec1bd70c 100644 --- a/src/backend/backup/basebackup_incremental.c +++ b/src/backend/backup/basebackup_incremental.c @@ -726,7 +726,9 @@ GetIncrementalFilePath(Oid dboid, Oid spcoid, RelFileNumber relfilenumber, * How should we back up a particular file as part of an incremental backup? * * If the return value is BACK_UP_FILE_FULLY, caller should back up the whole - * file just as if this were not an incremental backup. + * file just as if this were not an incremental backup. The + * relative_block_numbers array might be clobbered, but should not be + * consulted. * * If the return value is BACK_UP_FILE_INCREMENTALLY, caller should include * an incremental file in the backup instead of the entire file. On return, @@ -745,7 +747,6 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path, BlockNumber *relative_block_numbers, unsigned *truncation_block_length) { - BlockNumber absolute_block_numbers[RELSEG_SIZE]; BlockNumber limit_block; BlockNumber start_blkno; BlockNumber stop_blkno; @@ -872,8 +873,13 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path, errcode(ERRCODE_INTERNAL_ERROR), errmsg_internal("overflow computing block number bounds for segment %u with size %zu", segno, size)); + + /* + * This will write *absolute* block numbers into the output array, but + * we'll transpose them below. + */ nblocks = BlockRefTableEntryGetBlocks(brtentry, start_blkno, stop_blkno, - absolute_block_numbers, RELSEG_SIZE); + relative_block_numbers, RELSEG_SIZE); Assert(nblocks <= RELSEG_SIZE); /* @@ -892,19 +898,22 @@ GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path, return BACK_UP_FILE_FULLY; /* - * Looks like we can send an incremental file, so sort the absolute the - * block numbers and then transpose absolute block numbers to relative - * block numbers. + * Looks like we can send an incremental file, so sort the block numbers + * and then transpose them from absolute block numbers to relative block + * numbers if necessary. * * NB: If the block reference table was using the bitmap representation * for a given chunk, the block numbers in that chunk will already be * sorted, but when the array-of-offsets representation is used, we can * receive block numbers here out of order. */ - qsort(absolute_block_numbers, nblocks, sizeof(BlockNumber), + qsort(relative_block_numbers, nblocks, sizeof(BlockNumber), compare_block_numbers); - for (i = 0; i < nblocks; ++i) - relative_block_numbers[i] = absolute_block_numbers[i] - start_blkno; + if (start_blkno != 0) + { + for (i = 0; i < nblocks; ++i) + relative_block_numbers[i] -= start_blkno; + } *num_blocks_required = nblocks; /* -- 2.39.3 (Apple Git-146)