Re: UBSan pointer overflow in xlogreader.c - Mailing list pgsql-hackers

From Robert Haas
Subject Re: UBSan pointer overflow in xlogreader.c
Date
Msg-id CA+Tgmoat59LxYc3Yg6Rm9V=2Q3egfyyqouK_kcnea+QyBd9OdQ@mail.gmail.com
Whole thread Raw
In response to Re: UBSan pointer overflow in xlogreader.c  (Nathan Bossart <nathandbossart@gmail.com>)
Responses Re: UBSan pointer overflow in xlogreader.c
List pgsql-hackers
On Tue, Dec 5, 2023 at 1:04 PM Nathan Bossart <nathandbossart@gmail.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



pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Add checks in pg_rewind to abort if backup_label file is present
Next
From: Andrew Dunstan
Date:
Subject: Re: Emitting JSON to file using COPY TO