Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2) - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)
Date
Msg-id 20120725203816.GA21271@momjian.us
Whole thread Raw
In response to Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Wed, Jul 25, 2012 at 04:09:42PM -0400, Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > On Thu, Jul 19, 2012 at 02:24:36AM -0400, Bruce Momjian wrote:
> >> On Wed, Jul 18, 2012 at 01:30:34AM -0400, Tom Lane wrote:
> >>> Some googling suggests that on Solaris, this calculation would actually
> >>> give a result that's a little too *large* not too small, because of
> >>> padding in the declaration of struct dirent.  That would not be a
> >>> problem so far as callers were concerned, but it's conceivable that the
> >>> memcpy in load_directory would run off the end of memory and crash.
> 
> > This bug report was fixed mostly via private email because a private
> > schema dump was sent to Tom and me.  Tom fixed the problem by changing
> > the way we interact with struct dirent, but neither of us is sure
> > exactly why the fix worked.  It is something about Solaris.
> 
> I'm quite sure that the explanation is as above, that is, we were
> computing an incorrectly large value for the length of the directory
> entry, and that ran off the end of a memory-mapped directory page.
> The actual length of the entry, for a five-character file name,
> would be offsetof(d_name) plus 6 bytes, which assuming the entry
> requires 4-byte alignment would pad to offsetof(d_name) plus 8;
> but we were computing offsetof(d_name) plus 9 bytes, just enough
> to try to fetch a byte that wasn't there.

OK.

> > Tom suggested that load_directory() return a (char *) array, rather than
> > a struct dirent array, greatly simplifying the code.
> > I have done this in the attached patch, and because of the uncertainty
> > of ths fix, I would like to apply this to 9.2 as well.
> 
> I think patching 9.2 is reasonable, but on the grounds of keeping it
> parallel to HEAD, not that we don't know why the previous code was
> broken.

Agreed.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)
Next
From: Bruce Momjian
Date:
Subject: Re: Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)