Thread: Invalid memory access in pg_stat_get_subscription

Invalid memory access in pg_stat_get_subscription

From
Kuntal Ghosh
Date:
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

Attachment

Re: Invalid memory access in pg_stat_get_subscription

From
Tom Lane
Date:
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



Re: Invalid memory access in pg_stat_get_subscription

From
Kuntal Ghosh
Date:
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