Invalid memory access in pg_stat_get_subscription

Started by Kuntal Ghoshalmost 4 years ago3 messageshackers
Jump to latest
#1Kuntal Ghosh
kuntalghosh.2007@gmail.com

Hello hackers,

While exploring some code in logical replication worker
implementation, I noticed that we're accessing an invalid memory while
traversing LogicalRepCtx->workers[i].
For the above structure, we're allocating
max_logical_replication_workers times LogicalRepWorker amount of
memory in ApplyLauncherShmemSize. But, in the for loop, we're
accessing the max_logical_replication_workers + 1 location which is
resulting in random crashes.

Please find the patch that fixes the issue. I'm not sure whether we
should add a regression test for the same.

--
Thanks & Regards,
Kuntal Ghosh

Attachments:

0001-Fix-terminating-condition-while-fetching-the-replica.patchapplication/octet-stream; name=0001-Fix-terminating-condition-while-fetching-the-replica.patchDownload+1-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kuntal Ghosh (#1)
Re: Invalid memory access in pg_stat_get_subscription

Kuntal Ghosh <kuntalghosh.2007@gmail.com> writes:

While exploring some code in logical replication worker
implementation, I noticed that we're accessing an invalid memory while
traversing LogicalRepCtx->workers[i].
For the above structure, we're allocating
max_logical_replication_workers times LogicalRepWorker amount of
memory in ApplyLauncherShmemSize. But, in the for loop, we're
accessing the max_logical_replication_workers + 1 location which is
resulting in random crashes.

I concur that that's a bug, but eyeing the code, it seems like an
actual crash would be improbable. Have you seen one? Can you
reproduce it?

Please find the patch that fixes the issue. I'm not sure whether we
should add a regression test for the same.

How would you make a stable regression test for that?

regards, tom lane

#3Kuntal Ghosh
kuntalghosh.2007@gmail.com
In reply to: Tom Lane (#2)
Re: Invalid memory access in pg_stat_get_subscription

Hello Tom,

On Wed, Jun 8, 2022 at 12:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kuntal Ghosh <kuntalghosh.2007@gmail.com> writes:

While exploring some code in logical replication worker
implementation, I noticed that we're accessing an invalid memory while
traversing LogicalRepCtx->workers[i].
For the above structure, we're allocating
max_logical_replication_workers times LogicalRepWorker amount of
memory in ApplyLauncherShmemSize. But, in the for loop, we're
accessing the max_logical_replication_workers + 1 location which is
resulting in random crashes.

I concur that that's a bug, but eyeing the code, it seems like an
actual crash would be improbable. Have you seen one? Can you
reproduce it?

Thank you for looking into it. Unfortunately, I'm not able to
reproduce the crash, but I've seen one crash while executing the
function. The crash occurred at the following line:

if (!worker.proc || !IsBackendPid(worker.proc->pid))

(gdb) p worker.proc
$6 = (PGPROC *) 0x2bf0b9
The PGPROC structure was pointing to an invalid memory location.

--
Thanks & Regards,
Kuntal Ghosh