Potential stack overflow in incremental base backup

Поиск
Список
Период
Сортировка
От Thomas Munro
Тема Potential stack overflow in incremental base backup
Дата
Msg-id CA+hUKG+2hZ0sBztPW4mkLfng0qfkNtAHFUfxOMLizJ0BPmi5+g@mail.gmail.com
обсуждение исходный текст
Ответы Re: Potential stack overflow in incremental base backup  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Re: Potential stack overflow in incremental base backup  (Robert Haas <robertmhaas@gmail.com>)
Список pgsql-hackers
Hi Robert,

I was rebasing my patch to convert RELSEG_SIZE into an initdb-time
setting, and thus runtime variable, and I noticed this stack variable
in src/backend/backup/basebackup_incremental.c:

GetFileBackupMethod(IncrementalBackupInfo *ib, const char *path,
                                        BlockNumber *relative_block_numbers,
                                        unsigned *truncation_block_length)
{
    BlockNumber absolute_block_numbers[RELSEG_SIZE];

I'll have to move that sucker onto the heap (we banned C99 variable
length arrays and we don't use nonstandard alloca()), but I think the
coding in master is already a bit dodgy: that's 131072 *
sizeof(BlockNumber) = 512kB with default configure options, but
--with-segsize X could create a stack variable up to 16GB,
corresponding to segment size 32TB (meaning no segmentation at all).
That wouldn't work.  Shouldn't we move it to the stack?  See attached
draft patch.

Even on the heap, 16GB is too much to assume we can allocate during a
base backup.  I don't claim that's a real-world problem for
incremental backup right now in master, because I don't have any
evidence that anyone ever really uses --with-segsize (do they?), but
if we make it an initdb option it will be more popular and this will
become a problem.  Hmm.

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: shveta malik
Дата:
Сообщение: Re: Missing LWLock protection in pgstat_reset_replslot()
Следующее
От: Michael Paquier
Дата:
Сообщение: Re: Fix race condition in InvalidatePossiblyObsoleteSlot()