Re: Allow non-superuser to cancel superuser tasks.

On Thu, Apr 11, 2024 at 04:55:59PM +0500, Kirill Reshke wrote:
>> It's feeling more natural here to check that we have a superuser-owned
>> backend first, and then do a lookup of the process type.
>
>> I figured since there's no reason to rely on that behavior, we might as
>> well do a bit of future-proofing in case autovacuum workers are ever not
>> run as InvalidOid.
>
> I have combined them into this:
>
> if ((!OidIsValid(proc->roleId) || superuser_arg(proc->roleId))
> && pgstat_get_backend_type(GetNumberFromPGProc(proc)) == B_AUTOVAC_WORKER)
>
> This is both future-proofing and natural, I suppose. Downside of this
> is double checking condition (!OidIsValid(proc->roleId) ||
> superuser_arg(proc->roleId)), but i think that is ok for the sake of
> simplicity.

If we want to retain the check, IMO we might as well combine the first two
blocks like Anthony proposed:

if (!OidIsValid(proc->roleId) || superuser_arg(proc->roleId))
{
ProcNumber procNumber = GetNumberFromPGProc(proc);
PGBackendStatus procStatus = pgstat_get_beentry_by_proc_number(procNumber);

if (procStatus && procStatus->st_backendType == B_AUTOVAC_WORKER &&
!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVAC_WORKER))
return SIGNAL_BACKEND_NOAUTOVAC;
else if (!superuser())
return SIGNAL_BACKEND_NOSUPERUSER;
}

+ <row>
+ <entry>pg_signal_autovacuum_worker</entry>
+ <entry>Allow signaling autovacuum worker backend to cancel or terminate</entry>
+ </row>

I think we need to be more specific about what pg_cancel_backend() and
pg_terminate_backend() do for autovacuum workers. The code offers some
clues:

/*
* SIGINT is used to signal canceling the current table's vacuum; SIGTERM
* means abort and exit cleanly, and SIGQUIT means abandon ship.
*/
pqsignal(SIGINT, StatementCancelHandler);
pqsignal(SIGTERM, die);

+/* ----------
+ * pgstat_get_backend_type() -
+ *
+ * Return the backend type of the backend for the given proc number.
+ * ----------
+ */
+BackendType
+pgstat_get_backend_type(ProcNumber procNumber)
+{
+ PgBackendStatus *ret;
+
+ ret = pgstat_get_beentry_by_proc_number(procNumber);
+
+ if (!ret)
+ return B_INVALID;
+
+ return ret->st_backendType;
+}

I'm not sure we really need to introduce a new function for this. I
avoided using it in my example snippet above. But, maybe it'll come in
handy down the road...

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com