Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Mike Wilson <mfwilson(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)
Date: 2012-07-25 20:09:42
Message-ID: 2736.1343246982@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Bruce Momjian <bruce(at)momjian(dot)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.

> 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.

regards, tom lane

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2012-07-25 20:30:10 Re: Re: [BUGS] BUG #6733: All Tables Empty After pg_upgrade (PG 9.2.0 beta 2)
Previous Message Bert Thomas 2012-07-25 18:39:03 Re: BUG #6761: unexpected behaviour of 'now'::timestamp

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-07-25 20:25:36 Re: filenames in pg_basebackup
Previous Message Alvaro Herrera 2012-07-25 19:37:09 filenames in pg_basebackup