1
0
mirror of https://github.com/postgres/postgres.git synced 2025-07-03 20:02:46 +03:00

Make get_controlfile not leak file descriptors

When backend functions were added to expose controldata via SQL,
reading of pg_control was consolidated under src/common so that
both frontend and backend could share the same code. That move
from frontend-only to shared frontend-backend failed to recognize
the risk (and coding standards violation) of using a bare open().
In particular, it risked leaking file descriptors if transient
errors occurred while reading the file. Fix that by using
OpenTransientFile() instead in the backend case, which is
purpose-built for this type of usage.

Since there have been no complaints from the field, and an intermittent
failure low risk, no backpatch. Hard failure would of course be bad, but
in that case these functions are probably the least of your worries.

Author: Joe Conway
Reviewed-By: Michael Paquier
Reported by: Michael Paquier
Discussion: https://postgr.es/m/20190227074728.GA15710@paquier.xyz
This commit is contained in:
Joe Conway
2019-02-28 15:57:40 -05:00
parent f414abd62d
commit 4598a99cf2

View File

@ -27,9 +27,12 @@
#include "catalog/pg_control.h" #include "catalog/pg_control.h"
#include "common/controldata_utils.h" #include "common/controldata_utils.h"
#include "port/pg_crc32c.h" #include "port/pg_crc32c.h"
#ifndef FRONTEND
#include "storage/fd.h"
#endif
/* /*
* get_controlfile(char *DataDir, const char *progname, bool *crc_ok_p) * get_controlfile()
* *
* Get controlfile values. The result is returned as a palloc'd copy of the * Get controlfile values. The result is returned as a palloc'd copy of the
* control file data. * control file data.
@ -51,13 +54,14 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
ControlFile = palloc(sizeof(ControlFileData)); ControlFile = palloc(sizeof(ControlFileData));
snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir); snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
#ifndef FRONTEND #ifndef FRONTEND
if ((fd = OpenTransientFile(ControlFilePath, O_RDONLY | PG_BINARY)) == -1)
ereport(ERROR, ereport(ERROR,
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not open file \"%s\" for reading: %m", errmsg("could not open file \"%s\" for reading: %m",
ControlFilePath))); ControlFilePath)));
#else #else
if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
{ {
fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"), fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
progname, ControlFilePath, strerror(errno)); progname, ControlFilePath, strerror(errno));
@ -95,7 +99,11 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
#endif #endif
} }
#ifndef FRONTEND
CloseTransientFile(fd);
#else
close(fd); close(fd);
#endif
/* Check the CRC. */ /* Check the CRC. */
INIT_CRC32C(crc); INIT_CRC32C(crc);