Fix for pg_stat_activity putting client hostaddr into appname field

From: Edmund Horner <ejrh00(at)gmail(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Fix for pg_stat_activity putting client hostaddr into appname field
Date: 2018-03-27 02:47:07
Message-ID: CAMyN-kA7aOJzBmrYFdXcc7Z0NmW+5jBaf_m=_-77uRNyKC9r=A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed when querying pg_stat_activity (in 10.1):

$ SELECT pid, application_name, client_hostname, backend_type FROM
pg_stat_activity;

pid | application_name | client_hostname | backend_type
-------+--------------------------+-----------------------+------------------------------
10207 | | | autovacuum launcher
10209 | | | logical
replication launcher
10211 | psql | localhost | client backend
10212 | DBeaver 4.3.4 - Main | ms006593.met.co.nz | client backend
10213 | DBeaver 4.3.4 - Metadata | ms006593.met.co.nz | client backend
10219 | psql | at-ice-db01.met.co.nz | client backend
10205 | ms006593.met.co.nz | | background writer
10204 | ms006593.met.co.nz | | checkpointer
10206 | at-ice-db01.met.co.nz | | walwriter

I've tracked this down to bootstrap/pgstat.c.

We create shared memory segments BackendAppnameBuffer,
BackendClientHostnameBuffer, and BackendActivityBufferSize with enough
room for MaxBackends strings each. In each PgBackendStatus item, we
point st_appname, st_clienthostname, and st_activity_raw to the
appropriate offset within this blocks.

But the stats array includes auxiliary processes, which means it has
NumBackendStatSlots items. The pointers for the aux process strings
are out of range. In the case of my query, the pointers for
st_appname in the aux processes happen to point into the
BackendClientHostnameBuffer segment.

This patch uses NumBackendStatSlots for the sizes of those segments,
rather than MaxBackends.

I considered whether aux processes really need those strings
(especially st_clienthostname), but decided it was more consistent
just to assume that they might. (It's an extra 3 kB... if we want to
save that we can put a bunch of if statements in pgstat_bestart and
other places.)

The patch also allocates local memory for st_clienthostname in
pgstat_read_current_status. These strings shouldn't change very
often, but it seems safer and more consistent to treat them as we
treat the other two string fields. Without the string copy, I think a
client disconnect and be replaced after the stats have been copied,
resulting in the new hostname appearing alongside the copied stats
fields from the old connection.

Cheers,
Edmund

Attachment Content-Type Size
pgstats_memory_sizes_v1.patch application/octet-stream 2.8 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haozhou Wang 2018-03-27 02:50:03 Re: [PATCH] Add missing type conversion functions for PL/Python
Previous Message Thomas Munro 2018-03-27 02:39:02 Re: Parallel safety of binary_upgrade_create_empty_extension