postgrespro.git / summary / log / commit / refs

commit    4598a99cf22de0dbab975d9c87ce16fd53146aca
Author:   Joe Conway <mail@joeconway.com>
Date:     Thu Feb 28 20:57:40 2019 +0000

    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


 src/common/controldata_utils.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
index abfe7065f5..000f3c66d6 100644
--- a/src/common/controldata_utils.c
+++ b/src/common/controldata_utils.c
@@ -27,9 +27,12 @@
 #include "catalog/pg_control.h"
 #include "common/controldata_utils.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
  * control file data.
@@ -51,13 +54,14 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
 	ControlFile = palloc(sizeof(ControlFileData));
 	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
 
-	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
 #ifndef FRONTEND
+	if ((fd = OpenTransientFile(ControlFilePath, O_RDONLY | PG_BINARY)) == -1)
 		ereport(ERROR,
 				(errcode_for_file_access(),
 				 errmsg("could not open file \"%s\" for reading: %m",
 						ControlFilePath)));
 #else
+	if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
 	{
 		fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
 				progname, ControlFilePath, strerror(errno));
@@ -95,7 +99,11 @@ get_controlfile(const char *DataDir, const char *progname, bool *crc_ok_p)
 #endif
 	}
 
+#ifndef FRONTEND
+	CloseTransientFile(fd);
+#else
 	close(fd);
+#endif
 
 	/* Check the CRC. */
 	INIT_CRC32C(crc);


[parent: f414abd62da7]