Re: pg_terminate_backend and pg_cancel_backend by not administrator user - Mailing list pgsql-hackers

From Josh Kupershmidt
Subject Re: pg_terminate_backend and pg_cancel_backend by not administrator user
Date
Msg-id BANLkTinoLXBjX8zOci=5cCcLL0VNAvPU+w@mail.gmail.com
Whole thread Raw
In response to Re: pg_terminate_backend and pg_cancel_backend by not administrator user  (Noah Misch <noah@leadboat.com>)
Responses Re: pg_terminate_backend and pg_cancel_backend by not administrator user  (Noah Misch <noah@leadboat.com>)
List pgsql-hackers
On Sun, May 29, 2011 at 5:04 AM, Noah Misch <noah@leadboat.com> wrote:
> What risks arise from unconditionally allowing these calls for the same user's
> backends?  `pg_cancel_backend' ought to be safe enough; the user always has
> access to the standard cancellation protocol, making the SQL interface a mere
> convenience (albeit a compelling one).  `pg_terminate_backend' does open up
> access to a new behavior, but no concrete risks come to mind.

Looking around, I see there were real problems[1] with sending SIGTERM
to individual backends back in 2005 or so, and pg_terminate_backend()
was only deemed safe enough to put in for 8.4 [2]. So expanding
pg_terminate_backend() privileges does make me a tad nervous.

Reading through those old threads made me realize this patch would
give database owners the ability to kill off autovacuum workers. Seems
like we'd want to restrict that power to superusers.

> On the other hand, this *would* be substantial new authority for database
> owners.  Seems like a reasonable authority to grant, though.

And I also realized that this patch's approach might force us to
maintain a permissions wart if we ever want to implement fine-grained
control for this stuff, e.g. a per-role setting enabling self-kills.
It would be a bit lame to have to document "Use this CREATE/ALTER ROLE
flag. Or be the database owner. Or be a superuser."

>> Now, a few technical comments about the patch:
>> 1.) This bit looks dangerous:
>> +                backend = pgstat_fetch_stat_beentry(i);
>> +                if (backend->st_procpid == pid) {
>>
>> Since pgstat_fetch_stat_beentry() might return NULL.
>
> I think you want BackendPidGetProc().

Ah, thanks for the pointer.

Josh

--
[1] http://postgresql.1045698.n5.nabble.com/pg-terminate-backend-idea-td1930120.html
[2]
http://postgresql.1045698.n5.nabble.com/Re-COMMITTERS-pgsql-Add-pg-terminate-backend-to-allow-terminating-only-a-single-td1983763i20.html


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [ADMIN] pg_class reltuples/relpages not updated by autovacuum/vacuum
Next
From: Tom Lane
Date:
Subject: Re: Getting a bug tracker for the Postgres project