On Tue, Aug 16, 2022 at 4:14 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> On Tue, Aug 16, 2022 at 1:16 PM Andres Freund <andres@anarazel.de> wrote:
> > > But let's suppose we want to play by a timid interpretation of that page's
> > > "do not issue low-level or STDIO.H I/O routines". It also says that SIGINT
> > > is special and runs the handler in a new thread (in a big warning box
> > > because that has other hazards that would break other kinds of code). Well,
> > > we *know* it's safe to unlink files in another thread... so... how cheesy
> > > would it be if we just did raise(SIGINT) in the real handlers?
> >
> > Not quite sure I understand. You're proposing to raise(SIGINT) for all other
> > handlers, so that signal_remove_temp() gets called in another thread, because
> > we assume that'd be safe because doing file IO in other threads is safe? That
> > assumes the signal handler invocation infrastructure isn't the problem...
>
> That's what I was thinking about, yeah. But after some more reading,
> now I'm wondering if we'd even need to do that, or what I'm missing.
> The 6 listed signals in the manual are SIGABRT, SIGFPE, SIGILL,
> SIGINT, SIGSEGV and SIGTERM (the 6 required by C). We want to run
> signal_remove_temp() on SIGHUP (doesn't exist, we made it up), SIGINT,
> SIGPIPE (doesn't exist, we made it up), and SIGTERM (exists for C spec
> compliance but will never be raised by the system according to the
> manual, and we don't raise it ourselves IIUC). So the only case we
> actually have to consider is SIGINT, and SIGINT handlers run in a
> thread, so if we assume it is therefore exempt from those
> very-hard-to-comply-with rules, aren't we good already? What am I
> missing?
I converted that analysis into a WIP patch, and tried to make the
Windows test setup as similar to Unix as possible. I put in the
explanation and an assertion that it's running in another thread.
This is blind coded as I don't have Windows, but it passes CI. I'd
probably need some help from a Windows-enabled hacker to go further
with this, though. Does the assertion hold if you control-C the
regression test, and is there any other way to get it to fail?
The next thing is that the security infrastructure added by commit
f6dc6dd5 for CVE-2014-0067 is ripped out (because unreachable) by the
attached, but the security infrastructure added by commit be76a6d3
probably doesn't work on Windows yet. Where src/port/mkdtemp.c does
mkdir(name, 0700), I believe Windows throws away the mode and makes a
default ACL directory, probably due to the mismatch between the
permissions models. I haven't studied the Windows security model, but
reading tells me that AF_UNIX will obey filesystem ACLs, so I think we
should be able to make it exactly as secure as Unix if we use native
APIs. Perhaps we just need to replace the mkdir() call in mkdtemp.c
with CreateDirectory(), passing in a locked-down owner-only
SECURITY_DESCRIPTOR, or something like that?
