Re: Allow non-superuser to cancel superuser tasks.

On Fri, 22 Nov 2024 at 22:13, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2024-07-09 13:12:59 -0500, Nathan Bossart wrote:
> > I've committed 0001.
>
> I justed ended up looking at this code for some boring reason. One thing that
> has me worried a bit is that pg_signal_backend() now does
> pgstat_get_beentry_by_proc_number(), triggering a pgstat_read_current_status()
> further down.
>
> pgstat_read_current_status() can be fairly expensive, both in CPU and in
> memory. It copies each proc's activity strings, which each can be 1kB by
> default!
>
> The "saving grace" is that most of the time the pid will be sourced from
> pg_stat_activity, which already will will have done
> pgstat_read_current_status(). But I don't think that's always the case.
>
>
> Another concern is that pgstat_read_current_status() won't actually refresh
> the information if already cached, which might cause us to operate on outdated
> information. A malicious user could start a transaction, cause
> pgstat_read_current_status() to be called, and wait for the PID to be reused
> for some interesting process, and then signal, which the outdated information
> from the prior pgstat_read_current_status() would allow.
>
> The threat of this isn't huge, there's some fundamental raciness around
> pg_signal_backend() but this does open the window a lot wider.
>
>
> And all of this just because we need the target proc's BackendType, a single 4
> byte value.
>
> I think it'd be better to introduce something that fetches a live
> BackendType. We have such functionality already, see
> pgstat_get_backend_current_activity().
>
> Or we could have a copy of the backend type in PGPROC.
>
> Greetings,
>
> Andres Freund

Hi, thanks for taking care of this. I agree with this analysis, and it
appears that the worries are legitimate.
For the fix, I believe that copy-pasting
`pgstat_get_backend_current_activity` to get the backend type should
solve the issue. Not sure if this is the correct way of doing this.
Enlarging PGPROC somehow feels worse.

--
Best regards,
Kirill Reshke