Re: UBSan pointer overflow in xlogreader.c

On Tue, Dec 5, 2023 at 1:04 PM Nathan Bossart <nathandbossart(at)gmail(dot)com> wrote:
> On Wed, Dec 06, 2023 at 12:03:53AM +1300, Thomas Munro wrote:
> > xlogreader.c has a pointer overflow bug, as revealed by the
> > combination of -fsanitize=undefined -m32, the new 039_end_of_wal.pl
> > test and Robert's incremental backup patch[1]. The bad code tests
> > whether an object could fit using something like base + size <= end,
> > which can be converted to something like size <= end - base to avoid
> > the overflow. See experimental fix patch, attached.
>
> The patch LGTM. I wonder if it might be worth creating some special
> pointer arithmetic routines (perhaps using the stuff in common/int.h) to
> help prevent this sort of thing in the future. But that'd require you to
> realize that your code is at risk of overflow, at which point it's probably
> just as easy to restructure the logic like you've done here.

The patch LGTM, too. Thanks for investigating and writing the code.
The part about how the reserved kernel memory prevents the bug from
appearing on 32-bit systems but not 64-bit systems running in 32-bit
mode is pretty interesting -- I don't want to think about how long it
would have taken me to figure that out.

--
Robert Haas
EDB: http://www.enterprisedb.com