Re: pg_terminate_backend and pg_cancel_backend by not administrator user

From: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
To: Noah Misch <noah(at)leadboat(dot)com>
Cc: Torello Querci <tquerci(at)gmail(dot)com>, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>, Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_terminate_backend and pg_cancel_backend by not administrator user
Date: 2011-05-29 14:56:02
Message-ID: BANLkTinoLXBjX8zOci=5cCcLL0VNAvPU+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, May 29, 2011 at 5:04 AM, Noah Misch <noah(at)leadboat(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-05-29 15:05:19 Re: Getting a bug tracker for the Postgres project
Previous Message Tom Lane 2011-05-29 14:40:27 Re: [ADMIN] pg_class reltuples/relpages not updated by autovacuum/vacuum