On 12/14/2011 05:24 AM, Magnus Hagander wrote:
How about passing a parameter to pg_signal_backend? Making
pg_signal_backend(int pid, int sig, bool allow_samerole)?
That works, got rid of the parts I didn't like and allowed some useful
minor restructuring. I also made the HINT better and match style
guidelines:
gsmith=> select pg_terminate_backend(21205);
ERROR: must be superuser to terminate other server processes
HINT: You can cancel your own processes with pg_cancel_backend().
gsmith=> select pg_cancel_backend(21205);
pg_cancel_backend
-------------------
t
New rev attached and pushed to
https://github.com/greg2ndQuadrant/postgres/tree/cancel-backend (which
is *not* the same branch as I used last time; don't ask why, long story)
I considered some additional ways to restructure the checks that could
remove a further line or two from the logic here, but they all made the
result seem less readable to me. And this is not a high performance
code path. I may have gone a bit too far with the comment additions
though, so feel free to trim that back. It kept feeling weird to me
that none of the individual signaling functions had their own intro
comments. I added all those.
I also wrote up a commentary on the PID wraparound race condition
possibility Josh brought up. Some research shows that pid assignment on
some systems is made more secure by assigning new ones randomly. That
seems like it would make it possible to have a pid get reused much
faster than on the usual sort of system that does sequential assignment
and wraparound. A reuse collision still seems extremely unlikely
though. With the new comments, at least a future someone who speculates
on this will know how much thinking went into the current
implementation: enough to notice, not enough to see anything worth
doing about it. Maybe that's just wasted lines of text?
With so little grief on the last round, I'm going to guess this one will
just get picked up by Magnus to commit next. Marking accordingly and
moved to the current CommitFest.
--
Greg Smith 2ndQuadrant US g...@2ndquadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
index e7f7fe0..cf77586 100644
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
*************** SELECT set_config('log_statement_stats',
*** 14244,14251 ****
<para>
The functions shown in <xref
linkend="functions-admin-signal-table"> send control signals to
! other server processes. Use of these functions is restricted
! to superusers.
</para>
<table id="functions-admin-signal-table">
--- 14244,14251 ----
<para>
The functions shown in <xref
linkend="functions-admin-signal-table"> send control signals to
! other server processes. Use of these functions is usually restricted
! to superusers, with noted exceptions.
</para>
<table id="functions-admin-signal-table">
*************** SELECT set_config('log_statement_stats',
*** 14262,14268 ****
<literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
</entry>
<entry><type>boolean</type></entry>
! <entry>Cancel a backend's current query</entry>
</row>
<row>
<entry>
--- 14262,14271 ----
<literal><function>pg_cancel_backend(<parameter>pid</parameter> <type>int</>)</function></literal>
</entry>
<entry><type>boolean</type></entry>
! <entry>Cancel a backend's current query. You can execute this against
! another backend that has exactly the same role as the user calling the
! function. In all other cases, you must be a superuser.
! </entry>
</row>
<row>
<entry>
*************** SELECT set_config('log_statement_stats',
*** 14304,14309 ****
--- 14307,14316 ----
<command>postgres</command> processes on the server (using
<application>ps</> on Unix or the <application>Task
Manager</> on <productname>Windows</>).
+ For the less restrictive <function>pg_cancel_backend</>, the role of an
+ active backend can be found from
+ the <structfield>usename</structfield> column of the
+ <structname>pg_stat_activity</structname> view.
</para>
<para>
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 7a2e0c8..1b7b75b 100644
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
***************
*** 30,35 ****
--- 30,36 ----
#include "postmaster/syslogger.h"
#include "storage/fd.h"
#include "storage/pmsignal.h"
+ #include "storage/proc.h"
#include "storage/procarray.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
*************** current_query(PG_FUNCTION_ARGS)
*** 70,84 ****
}
/*
! * Functions to send signals to other backends.
*/
static bool
! pg_signal_backend(int pid, int sig)
{
! if (!superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! (errmsg("must be superuser to signal other server processes"))));
if (!IsBackendPid(pid))
{
--- 71,113 ----
}
/*
! * Send a signal to another backend
*/
static bool
! pg_signal_backend(int pid, int sig, bool allow_same_role)
{
! PGPROC *proc;
! bool allowed = superuser();
!
! if (!allowed && allow_same_role)
! {
! /*
! * When same role permission is allowed, check for matching roles. Trust
! * that BackendPidGetProc will return NULL if the pid isn't valid, even
! * though the check for whether it's a backend process is below. The
! * IsBackendPid check can't be relied on as definitive even if it was
! * first. The process might end between successive checks regardless of
! * their order. There's no way to acquire a lock on an arbitrary
! * process to prevent that. But since so far all the callers of this
! * mechanism involve some request for ending the process anyway, that
! * it might end on its own first is not a problem.
! */
! proc = BackendPidGetProc(pid);
!
! if ((proc != NULL) && (proc->roleId == GetUserId()))
! allowed = true;
! else
! ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! (errmsg("must be superuser or have the same role to signal other server processes"))));
! }
!
! /* Rejected the same role case above, must be superuser only by here */
! if (!allowed)
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("must be superuser to terminate other server processes"),
! errhint("You can cancel your own processes with pg_cancel_backend().")));
if (!IsBackendPid(pid))
{
*************** pg_signal_backend(int pid, int sig)
*** 91,96 ****
--- 120,134 ----
return false;
}
+ /*
+ * Can the process we just validated above end, followed by the pid being
+ * recycled for a new process, before reaching here? Then we'd be trying to
+ * kill the wrong thing. Seems near impossible when sequential pid
+ * assignment and wraparound is used. Perhaps it could happen on a system
+ * where pid re-use is randomized. That race condition possibility seems
+ * too unlikely to worry about.
+ */
+
/* If we have setsid(), signal the backend's whole process group */
#ifdef HAVE_SETSID
if (kill(-pid, sig))
*************** pg_signal_backend(int pid, int sig)
*** 106,123 ****
return true;
}
Datum
pg_cancel_backend(PG_FUNCTION_ARGS)
{
! PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT));
}
Datum
pg_terminate_backend(PG_FUNCTION_ARGS)
{
! PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGTERM));
}
Datum
pg_reload_conf(PG_FUNCTION_ARGS)
{
--- 144,171 ----
return true;
}
+ /*
+ * Signal to cancel a backend process. This is allowed if you are superuser or
+ * have the same role as the process being canceled.
+ */
Datum
pg_cancel_backend(PG_FUNCTION_ARGS)
{
! PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGINT, true));
}
+ /*
+ * Signal to terminate a backend process. Only allowed by superuser.
+ */
Datum
pg_terminate_backend(PG_FUNCTION_ARGS)
{
! PG_RETURN_BOOL(pg_signal_backend(PG_GETARG_INT32(0), SIGTERM, false));
}
+ /*
+ * Signal to reload the database configuration
+ */
Datum
pg_reload_conf(PG_FUNCTION_ARGS)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
