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