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

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

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    g...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support  www.2ndQuadrant.us

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to