Re: Allow non-superuser to cancel superuser tasks.

Hi,

On 2024-11-22 13:21:53 -0600, Nathan Bossart wrote:
> Here is a draft-grade patch for this one. It seems pretty
> straightforward...

Thanks for looking into it quickly.

> diff --git a/src/backend/storage/ipc/signalfuncs.c b/src/backend/storage/ipc/signalfuncs.c
> index aa729a36e3..8e165e9729 100644
> --- a/src/backend/storage/ipc/signalfuncs.c
> +++ b/src/backend/storage/ipc/signalfuncs.c
> @@ -87,10 +87,7 @@ pg_signal_backend(int pid, int sig)
> */
> 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)
> + if (pgstat_get_backend_type(pid) == B_AUTOVAC_WORKER)
> {
> if (!has_privs_of_role(GetUserId(), ROLE_PG_SIGNAL_AUTOVACUUM_WORKER))
> return SIGNAL_BACKEND_NOAUTOVAC;

Because we already mapped the pid to a ProcNumber, it'd be cheaper to access
the backend status via procnumber.

> +BackendType
> +pgstat_get_backend_type(int pid)
> +{
> + PgBackendStatus *beentry = BackendStatusArray;
> +
> + for (int i = 1; i <= MaxBackends; i++)
> + {
> + volatile PgBackendStatus *vbeentry = beentry;
> + bool found;
> +
> + for (;;)
> + {
> + int before_changecount;
> + int after_changecount;
> +
> + pgstat_begin_read_activity(vbeentry, before_changecount);
> +
> + found = (vbeentry->st_procpid == pid);
> +
> + pgstat_end_read_activity(vbeentry, after_changecount);

We don't need the pgstat_begin_read_activity() protocol when just accessing a
single 4 byte value - we assume in lots of places that can be read in a
non-tearable way.

> + if (pgstat_read_activity_complete(before_changecount,
> + after_changecount))
> + break;
> +
> + CHECK_FOR_INTERRUPTS();
> + }
> +
> + if (found)
> + return beentry->st_backendType;

But if we were to follow it, we'd certainly need to use it here too.

Greetings,

Andres Freund