| From: | JoongHyuk Shin <sjh910805(at)gmail(dot)com> |
|---|---|
| To: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
| Cc: | "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: [PATCH] Fix TOCTOU race in ReplicationSlotsComputeRequiredLSN() |
| Date: | 2026-04-17 07:46:31 |
| Message-ID: | CACSdjfPtvQ9XQ5QLENHsofbPazqTT55Xufrq7XL_8QNEWtDJ1A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Zhijie,
Thanks for pointing this out.
You're right that my patch only covers the read side
and misses the write side in ReplicationSlotReserveWal().
I should have checked for existing work before submitting.
I'll withdraw this patch.
Best Regards,
JoongHyuk Shin
On Fri, Apr 17, 2026 at 3:59 PM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
wrote:
> On Friday, April 17, 2026 2:50 PM JoongHyuk Shin <sjh910805(at)gmail(dot)com>
> wrote:
> > Commit 2a5225b99d7 fixed a race in ReplicationSlotsComputeRequiredXmin()
> > where ReplicationSlotControlLock was released before the global xmin
> > update, allowing a concurrent backend to overwrite a correct value with
> > a stale one.
> >
> > ReplicationSlotsComputeRequiredLSN() has the same problem,
> > it releases the lock before calling XLogSetReplicationSlotMinimumLSN(),
> > so a stale minimum LSN can overwrite a correct (lower) one,
> > potentially leading to premature WAL removal.
> >
> > The attached patch moves LWLockRelease() to after the LSN update,
> > matching the xmin fix.
> > Since 2a5225b99d7 was backpatched to all supported versions,
> > I believe this should be as well.
>
> Thanks for noticing this. There is an existing thread [1] that I started
> following 2a5225b99d7 to address the same issue. The patch you posted
> only increases the lock scope in ReplicationSlotsComputeRequiredLSN() but
> does
> not increase the lock level when reserving WALs, so I think it would not
> fix the issue.
>
> Please feel free to review the patch in that thread if you find it helpful.
>
> [1] https://commitfest.postgresql.org/patch/6451/
>
> Best Regards,
> Hou zj
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-04-17 08:10:22 | Re: Row pattern recognition |
| Previous Message | Chao Li | 2026-04-17 07:40:40 | Re: Fix stats reporting delays in logical parallel apply worker |