Re: Re: Proposed Windows-specific change: Enable crash dumps (like core files)

On 16/12/10 21:01, Magnus Hagander wrote:

> Found another problem in it: when running with an older version of
> dbghelp.dll (which I was), it simply didn't work. We need to grab the
> version of dbghelp.dll at runtime and pick which things we're going to
> dump based on that.

I was about to suggest dropping the fallback loading of the system
dbghelp.dll, and just bundle a suitable version with Pg, then I saw how
big the DLL is. It's 800kb (!!) of code that'll hopefully never get used
on any given system. With a footprint like that, bundling it seems
rather less attractive.

I think Magnus is right: test the dbghelp.dll version and fall back to
supported features - as far back as XP, anyway; who cares about anything
earlier than that. An updated version can always be put in place by the
user if the built-in one can't produce enough detail.

> * I moved it all into backend/port/win32, and thus removed all the
> autoconf things, since they are not necessary. This is only for win32
> at this point, and AFAIK we don't expect it to be for other platforms.
> If that's needed, we can move it back later, but for now let's keep it
> simple.

Yeah, fair call - especially as most other OSes have native crash dump
facilities (kernel-generated core dumps, etc) that render explicit crash
handling much less useful.

> * using elog() really isn't safe. We set the crash handler *long*
> before we have loaded the elog subsystem. It's better to get a log to
> eventlog than to not get it at all.

Good point. It made some sense when running as a loadable module, as the
crash dumper was only loaded quite late, and so long as it elog()'d only
after writing the dump that was fine. Now, not so much.

> We use the existance of the "crashdumps" directory as an indication we
> want crashdumps. That's fine when the system is up. But what if we
> crash *in the postmaster before we have done chdir()*?
> 
> Should we perhaps instead define a subdirectory of *where the .EXE
> file is*, and dump the file there?

Well, on Windows it's pretty easy to find out where your executable is,
and the crash dumper already does that to determine where to load
dbghelp.dll from. It'd be no stretch to use a subdir of the postgresql
install dir for crash dumps. OTOH, there are all sorts of exciting
permissions issues to deal with then, as write permission for "postgres"
won't be inherited from the datadir. It's also generally a big ugly and
not very good practice to write to the application directory.

If not writing to the datadir, for non-services you'd use something like:
   %LOCALAPPDATA%\postgres\crashdumps

... but for a service running under its own account that's not a good
option either. Consequently, IMO the datadir remains the best place for
crashdumps.

Is this going to be a real-world issue? One simple answer might be not
to load the crash dump handler until after the postmaster chdir()s, but
honestly I'm not sure it matters. If it can't find somewhere to write
dumps, it doesn't write a dump.

The only possible issue I can imagine would be if the postmaster started
with a cwd that a malicious user could write to, so they could create a
'crashdumps' dir, somehow cause an early start postmaster crash,
therefore somehow leak info from the protected "postgres" account files
that way. A quick look at the code suggests that the postmaster doesn't
do anything interesting in the datadir before chdir()ing in there, so I
don't see any danger in that. In any case, if a malicious user has write
access to the postmaster's startup cwd, with the design of Windows PATH
and library loading that means they can replace critical DLLs with their
own malicious versions, so there's a whole lot more to worry about than
leaking information via crashdumps.

So: I say just use the cwd, whatever it is, and don't worry about it. An
admin can manually create a "crashdumps" dir in the service start cwd
and set appropriate permissions in the unlikely event they're trying to
collect crash dumps for an early-crashing postmaster and they can't just
use a debugger directly.

> It needs to be documented somewhere.Perhaps in "15.8 Platform Specific
> Notes"? That's really about building it, but it might be reasonable
> there anyway? It does hold a number of things today that aren't
> related to building, for other platforms.

Seems reasonable.

-- 
System & Network Administrator
POST Newspapers