From: Joe Conway <mail@joeconway.com>
To: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: get_controlfile() can leak fds in the backend
Date: Wed, 27 Feb 2019 19:45:11 -0500
Message-ID: <22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com> (raw)
In-Reply-To: <748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>
References: <20190227074728.GA15710@paquier.xyz>
<748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>
On 2/27/19 10:26 AM, Joe Conway wrote:
> On 2/27/19 2:47 AM, Michael Paquier wrote:
>> Hi all,
>> (CC-ing Joe as of dc7d70e)
> According to that comment BasicOpenFile does not seem to solve the issue
> you are pointing out (leaking of file descriptor on ERROR). Perhaps
> OpenTransientFile() is more appropriate? I am on the road at the moment
> so have not looked very deeply at this yet though.
It seems to me that OpenTransientFile() is more appropriate. Patch done
that way attached.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
Attachments:
[text/x-patch] controldata_utils-fdfix-01.diff (1.8K, ../22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com/2-controldata_utils-fdfix-01.diff)
download | inline diff:diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.cindex abfe706..c3b1934 100644
*** a/src/common/controldata_utils.c
--- b/src/common/controldata_utils.c
***************
*** 27,32 ****
--- 27,35 ----
#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(const char *DataDir, con
*** 51,63 ****
ControlFile = palloc(sizeof(ControlFileData));
snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
- if ((fd = open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) == -1)
#ifndef FRONTEND
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not open file \"%s\" for reading: %m",
ControlFilePath)));
#else
{
fprintf(stderr, _("%s: could not open file \"%s\" for reading: %s\n"),
progname, ControlFilePath, strerror(errno));
--- 54,67 ----
ControlFile = palloc(sizeof(ControlFileData));
snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);
#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));
*************** get_controlfile(const char *DataDir, con
*** 95,101 ****
--- 99,109 ----
#endif
}
+ #ifndef FRONTEND+ CloseTransientFile(fd);+ #else
close(fd);
+ #endif
/* Check the CRC. */
INIT_CRC32C(crc);
[application/pgp-signature] signature.asc (832B, ../22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com/3-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mail@joeconway.com, michael@paquier.xyz, pgsql-hackers@lists.postgresql.org
Subject: Re: get_controlfile() can leak fds in the backend
In-Reply-To: <22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox