Fix for pg_stat_activity putting client hostaddr into appname field - Mailing list pgsql-hackers

From Edmund Horner
Subject Fix for pg_stat_activity putting client hostaddr into appname field
Date
Msg-id CAMyN-kA7aOJzBmrYFdXcc7Z0NmW+5jBaf_m=_-77uRNyKC9r=A@mail.gmail.com
Whole thread Raw
Responses Re: Fix for pg_stat_activity putting client hostaddr into appname field
Re: Fix for pg_stat_activity putting client hostaddr into appnamefield
List 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

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Parallel safety of binary_upgrade_create_empty_extension
Next
From: Haozhou Wang
Date:
Subject: Re: [PATCH] Add missing type conversion functions for PL/Python