Re: doPickSplit stack buffer overflow in XLogInsert? - Mailing list pgsql-hackers

From Noah Misch
Subject Re: doPickSplit stack buffer overflow in XLogInsert?
Date
Msg-id 20131127171007.GA1086260@tornado.leadboat.com
Whole thread Raw
In response to Re: doPickSplit stack buffer overflow in XLogInsert?  (Kevin Grittner <kgrittn@ymail.com>)
Responses Re: doPickSplit stack buffer overflow in XLogInsert?  (Kevin Grittner <kgrittn@ymail.com>)
Re: doPickSplit stack buffer overflow in XLogInsert?  (Peter Geoghegan <pg@heroku.com>)
List pgsql-hackers
On Wed, Nov 27, 2013 at 06:23:38AM -0800, Kevin Grittner wrote:
> Andres Freund <andres@2ndquadrant.com> wrote:
> > On 2013-11-26 14:14:38 -0800, Kevin Grittner wrote:
> >
> >> I happened to build in a shell that was still set up for the clang
> >> address sanitizer, and got the attached report.  On a rerun it was

(Kevin, I saw no attachment.)

> >> repeatable.  XLogInsert() seems to read past the end of a variable
> >> allocated on the stack in doPickSplit(). I haven't tried to analyze
> >> it past that, since this part of the code is unfamiliar to me.
> >
> > Yea, I've seen that one before as well and planned to report it at some
> > point. The reason is the MAXALIGN()s in ACCEPT_RDATA_DATA(). That rounds
> > up to 8byte boundaries, while we've e.g. only added 2bytes of slop to
> > toDelete.
> 
> Have you established whether having the CRC calculation read past
> the end of the buffer can cause problems on recovery or standby
> systems?  Should we try to get this fixed by Monday?

The threat is that rounding the read size up to the next MAXALIGN would cross
into an unreadable memory page, resulting in a SIGSEGV.  Every palloc chunk
has MAXALIGN'd size under the hood, so the excess read of "toDelete" cannot
cause a SIGSEGV.  For a stack variable, it depends on the ABI.  I'm not aware
of an ABI where the four bytes past the end of this stack variable could be
unreadable, which is not to claim I'm well-read on the topic.  We should fix
this in due course on code hygiene grounds, but I would not back-patch it.

-- 
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com



pgsql-hackers by date:

Previous
From: Boszormenyi Zoltan
Date:
Subject: Re: Modify the DECLARE CURSOR command tag depending on the scrollable flag
Next
From: Alvaro Herrera
Date:
Subject: Re: Errors on missing pg_subtrans/ files with 9.3