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
