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