On 12/15/2011 07:36 PM, Josh Kupershmidt wrote:
> The only niggling concern I have about this patch (and, I think, all
> similar ones proposed) is the possible race condition between the
> permissions checking and the actual call of kill() inside
> pg_signal_backend().
This is a problem with the existing code though, and the proposed
changes don't materially alter that; there's just another quick check in
one path through. Right now we check if someone is superuser, then if
it's a backend PID, then we send the signal. If you assume someone can
run through all the PIDs between those checks and the kill, the system
is already broken that way.
> I notice that BackendPidGetProc() has the comment:
>
> ... Note that it is up to the caller to be
> sure that the question remains meaningful for long enough for the
> answer to be used ...
>
> So someone has mulled this over before. It took my script maybe 15-20
> minutes to cause a wraparound by running fork() in a loop, and that
> wraparound would somehow have to occur within the short execution of
> pg_signal_backend().
Right, the the possibility you're raising is the obvious flaw with this
approach. Currently the only consumers of BackendPidGetProc or
IsBackendPid are these cancel/terminate functions. As you measured,
running through the PIDs fast enough to outrace any of that code is
unlikely. I'm sure a lot of other UNIX apps would break, too, if that
ever became untrue. The sort of thing that comment is warning is that
you wouldn't want to do something like grab a PID, vacuum a table, and
then assume that PID was still valid.
--
Greg Smith 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
