On 2014-05-06 13:33:01 +0300, Heikki Linnakangas wrote:
> On 03/31/2014 09:08 PM, Robert Haas wrote:
> >On Wed, Mar 26, 2014 at 9:45 PM, Peter Geoghegan <p...@heroku.com> wrote:
> >>On Wed, Nov 27, 2013 at 9:10 AM, Noah Misch <n...@leadboat.com> wrote:
> >>>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.
> >>
> >>Attached patch silences the "Invalid read of size n" complaints of
> >>Valgrind. I agree with your general thoughts around backpatching. Note
> >>that the patch addresses a distinct complaint from Kevin's, as
> >>Valgrind doesn't take issue with the invalid reads past the end of
> >>spgxlogPickSplit variables on the stack.
> >
> >Is the needless zeroing this patch introduces apt to cause a
> >performance problem?
> >
> >This function is actually pretty wacky.  If we're stuffing bytes with
> >undefined contents into the WAL record, maybe the answer isn't to
> >force the contents of those bytes to be defined, but rather to elide
> >them from the WAL record.
> Agreed. I propose the attached, which removes the MAXALIGNs. It's not
> suitable for backpatching, though, as it changes the format of the WAL
> record.

Not knowing anything about spgist this looks sane to me. Do we need a
backpatchable solution given that we seem to agree that these bugs
aren't likely to cause harm?


Andres Freund

 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to