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