Subject: Re: get_controlfile() can leak fds in the backend
From: Joe Conway <mail@joeconway.com>
To: Michael Paquier <michael@paquier.xyz>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
References: <20190227074728.GA15710@paquier.xyz>
 <748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>
Openpgp: preference=signencrypt
Autocrypt: addr=mail@joeconway.com; keydata=
 mQINBEpXMCsBEADDnXUQzjlyi/cX02Gtdy2CLcroE5CsC7DJKdOBDbfgn0kfiIYoV5JniG4l
 VyzZUodY8yUAagqLYolh0UkBzs9N+qkm7erde4ypw3jzVQ37BuzIvk3nMUbuDZDgxWqX+nVS
 sKc+BQ5BpzgCHg48leoRO2ohjvYnUhgH3j2rFZCzaj6qQ7mv+XoxOJmUlVQtG06Jwkk7Vu14
 7U9nMMM6hyUKzVnmCphnlcMNo26UyVU70MwFfFJgcI0c5fpp8byN56eD6VJVnufO5WAuEhzE
 qcrSJR2FAlmM90GBY+6vP29twLDCHuSFvrnujNCx/BvCC/a3/gPvyAFp4JtMm9eXAmq3m/Kw
 94nTJXVdcbQeQQDp3KIG7MmWS4lnGvPn8v0CjgNaLvZXFLo1FgmUVsyEq1Lww4iRLa6sbpXJ
 ESx15UEue1k1YZM9C+4F/o3aeKNsAienjw2EXFzcaxIg/C4P493VMi3Qa8ycVxR5iYhUbYdo
 DFIUQhbFNsYfrtW/qZAELT3FCYFpZYG01e9Hj+cBrXXgyDDkQ5Lq4mlvmkRvuxn61V6Au4HA
 0sJiCox5pM1FvzT+aI8HY1BYaiB9Pl4fhpKgmhhlSuglk9v39S4jmlUIb45iLAUVpeNM6Qjm
 69pf5da9sm4aGFa7YlDSKf/WcU7z9ITZxsilOi2n7YJiwG7kTQARAQABtB9Kb2UgQ29ud2F5
 IDxtYWlsQGpvZWNvbndheS5jb20+iQI8BBMBAgAmAhsDBwsJCAcDAgEGFQgCCQoLBBYCAwEC
 HgECF4AFAlpoksUCGQEACgkQMyt+aLaZQ0p4XQ/9F4WSf4ZFuL3ae356ApVqnLxphHXeXHli
 ePZe7qosvXMQLN9Y2m5cQwqojsQh4pZujIi4Iwxw7ORg6HbMO8dMhyu8TlDavErcgO561hG7
 /TCcCLf5SkadxBqYWAJH09W8S2iC3gQtS+qs4Y7sIvRi881EsPzG5Lrzu7Ei8ufqsSnbLUQk
 zAaKjK3IzAFvzA3VCg/8Aqhf8obhqL0j5I5DNOpAt2U4gkEzIrX1g5xSXpRLMHLEK7hPViY8
 pFQyRvyZioE4hckNsnidCgiv4HefJUFRIVxDkHBeLhTzqTPloesFT/bskhckIS6F4suMx044
 WTTW6UiARuWw3aaBTmxay2rjPCsGaJPawzY0O3HlNlZherCzuBpa+Qk1kMLkcxMlYjCrMgIR
 YyBpIK46ltPQaog4Ujh21w1Z8m/ufG5v/HTuLdXM9hGfTlW7aY6psCAV/MwfycDHLRMc+UY7
 W2cp6DlWQ7GHLP+vzvBcl5Odi04EhcYnHMZjKvRFsxhE0LZcTejrtxPkDgR9G59irlIx1+py
 ziK2B0hs7Er1Yq7qQGQiQGLoglSNDsvsUeyMGyabasmUd5xrRO35SpGv+hyYGAZ3Pyk9Vtxl
 CO7BJkZiV9YSMHp6hOdmYZd4SZRChd9sKd6eSw1xIlBALz1ZHuQVMBuZvuVGFsS81Zqckb/2
 D5q5Ag0ESlcx7QEQAKhm5UhGfbe3hM4yCaa/pPaNaswlB2tK1DHAjDlDm7gDVEodfsrvAFUU
 unLodlvCd0g/GpLmXXdROU/I+HusVqQhDbVeHMwO2VUVHamRWqDfrCjMYVxLz80d96TYyeyW
 6RUNkJmXJAOKgy8veI7Z1F+j1gXQR7UlzonnSkWbs0oQ3RglvyqhkUzWtXU7F+j2hQ2vQsct
 Lnq3lYo0bYkZ3K6RyKZXSly9Hd7DzYkMzz7StNRlyeQJxplPrVPup7N8SqDrI126sq0cwdUB
 Gn1sKVoO+bGidBjhJgxouNPkMJlPjbKOgb7tv5ZnM33FGXNdC5sPKoZ4mC+gaqRHYaH/V61v
 fCo1CYNLIK+Cj+Oua8mQ6CPeOb5VKcJ0JM96eGNlzF9EawppOG7xCDcbmBEr0m1go9MHEUel
 0L1i8RSDYOHQQkLT2MVWYf4UsRlEGbME+kF7DEX9Fvie7qglFTXC2d+YVN73AnXcoM/IYUOR
 DXjVVaqQoHuA4uxwJ31o/rEaOihr2yCScr8mB8J3b3KyNfuCBm5gN0tUQw+t5szMYurDpZ1G
 L4O7btaQ5u4iJMfRTCnPrzW+wkdVrao7AmRNWEtSlXsycClKFCHz/Ej5rpXOKSp+a/xcaFX1
 v2eS/+Ujp5ESu9p57JwlP6aSqYBiOALRJXZ9DVtZL96okjzXjrd7ABEBAAGJBD4EGAEIAAkF
 AkpXMe0CGwICKQkQMyt+aLaZQ0rBXSAEGQEIAAYFAkpXMe0ACgkQN/L3QzX32GV1Jw//fhvL
 Mk1YNS/2N5jc5HYmEjcL2OWBLLrO5RMCaOgCNmYsepkigy2U+GSloVy8Y40/UfYzIoNf1DiB
 Qyj5Ipa4eMjfKA7+1q43a5wkDDWogFNflKTTNemwrHDKR1WWDrxt8uAgJdlBGDQA7A0q3D2i
 Xhd7ALLOHhuXDY1++N9TK9KMfV90Oay0mIMLQhTG/UgsgrYYb78H2pk8XMYZY29vgZlj9VH2
 f1RpOoh7lCaB+2glN27Rzh17q0/zEjwXk4sDyYHRIycaqnM4ozSnliXyoWM5bvmqSGBCm5jl
 BjNW+cFbFy15U595AIXXiUXl8hsTF2JWwgHr+hYdtoEH3oEOxzM18ENXKTTgN+BafdMsah4e
 HIcVvrVrH6rlsPwCvk3o/PJ07qVbUW+dbZG8UB1JwqUUqGhrCk87kztajlvZGZWxWDSB4Lnb
 T6Jc0L7/YXG8U3sJ2OtZ95Lal5XfNo0yBdPflmalH3xf/EcxaYFETbDrY5UgEVa2HBw8XzKk
 rTbM8rD318M+jBOuL6o3cGMOeFOm4C51fJZmOCFd03LhMkoj9IbLIPGKosIBsxb7zP5YLAbI
 b3el6pvo9CKtNuR+IcVOoFtH+KVOxGsbbxFEFbsAmPMreF1hIv+l+HXBaDhUkaUmpJ1Mhxol
 aM6O/LGG4VQV7wBQFunVaN8EPqMqYDph3A//TjQVd8fqvw5Ezj1958IUmV0jxeGX2m/fspGh
 njF+6Adx3PFbZoKyW71cA/TnlHVYCaizjYYRNMC3g5VFytOYOZFi0GKsxQTX5PcPhDjFFTUv
 FVVZwDYUK3QlC+wGDTeNWfAJIZnTPPIXSiHzh3si+ngKjE+ngcRF1VY5K4xvc6ss6uq8kI8L
 Ct+TpOMr99ijaYEbMZ8WJagynD5BLNYxpP6CCTMDi1Xqydg0E/mhR9w9aUOvdTZw1OOKtLnz
 B1E0wFbzJXEvjP2eEVNlStS5RTawFDpqxGwXJcHqe8qtg8B+C1r47RmHnKKm+DTOPFQWTj83
 l775aNMzAHm+Pr1TdSA5XRyQ3wxwYKfhp1suKhILUZLjbiCQf92RZQ6yEdXskMr2A4n0GQWq
 Q8k7r9QPqtUP7lsiyyDem7r9h5mAm94r3Td22L3JaSHkBD+4jC4gIXeYoyd+lTz/zYOx0Fri
 J8dXHalmyTGsSxV7FXv1/thVEf6R7TDdVwhqf/WL3uWTmVBR0tJscG+9iCVch/OG/uCDkWZQ
 sOTotqj6f5jSZI+NOUqt62+8i8gjYEa7zRZ2HXFOTV8UduCjheWB2VCUj5EjFjTMjfTVkyZ5
 SiFht7bM5l/odoYU6vn37/5s/ZsMTZX1zG5oXv6iULrHLYHhUxR93FlVyn4DiHsWND/Idj65
 Ag0ESlcyJwEQAOkTBb9yDhJbMUgvhM11rZwT5tm4Y9TqtEHn0Zy3t9g7bdFFpMvav/KENd3o
 AtLFpMDf+H3AggFk4ftUwJwiVgJ88ilvCynJUGXiuYIaexY4DLgn4xpnuiEpYEFVdWnlw7dW
 VTc62exfqIz9bSWRzwfBCY9ruYGEb4RDPDSNSAVyI7sxHzef2asiYxIcxrTrw5VugWNlPZcV
 5/EJ6PUvATjBF2TBkXV7KOciQng2tsQGrGMkY5mduNqwpuh6zfPcVF8LeObe96wv5ZhPRpO7
 9nef7hnK2lJogp3JIo558Jlbz9WHtQEMZR85+bUhtI825QyNAFz3Jrn7NMgvDikc2OrWo7YM
 gMC5hDSWVFqA6/EQCNnDWGABWgeYHZFpnPwsvUWIYdhSilUuj/Tuzvz9ZmucFNbQbauDQw6V
 Q38ofGnoYDZFJsGncprB8dBi4tDrIQ+1RlIh6C2Z/eMipqJOT26+spluTjouvnKT0S5yOgyX
 0PjbsysgwQdCGNJLHOjhHbSpSmOLaduV3CQo/0+DHT/TBjYfIXjTWouY9TkGxG4eNrxU0u2x
 Ay5bMqOPmsFdjLTWlQUlF/fTMhB54XwI3FHWgnSnXZzStDTmTebLNdT/ftgliAzA81uMj49j
 0exv731/v+7udLA1bV8gnZ01zQCASDpWiRQR3fgwcugSUqgRABEBAAGJAh8EGAEIAAkFAkpX
 MicCGwwACgkQMyt+aLaZQ0pwAQ//bjcWnZg/jjRQ9gbZUGMqniItZYRglBMKIqt4Fia379Jm
 HwTvavnFkJ8XMZ56UB0FIrgS+sUkRH6cPRQR+7Qi392LD021DXgSsz9CwFHjFyBGHwLEOTRc
 fYQbtJy0shHDJB4aQTOX3ERDH1PsvJNuevmQMzS0DWFav9+xMz9rKP4N+HffoBIZE0C1xIE4
 3nD4eLsbycte9sVIrmlNuUti3qUxJAQw8HwfJ6ZbBInHxquApR16uD1u99o6XlndFrDlY22t
 RmHCM0bR81GfGNdcU3Uo+rG/R/k4qa7s9/dgKvMbyH3fHhp/ceKag80Xo8IFurRl0ZJP3sHJ
 2QDHCVLat7jRZ+43hi1WlIhFbrgn6IyI0i7XR/W8JjrC5MsKq4TUwGH077sU/kcHYebVJZRb
 UUst2hAGHDFVBcG12qoKf+ltL9qXJc1y7BGeCoUW6QjOpljpq6ZL4FQUsM0RSRjs5egE3szP
 cIf5SyPK6WDOApoAq6M7BBFMGDZwEylYMtr0YekA1u86UA9D2xwLHEbBBp/uiby1c9JbPJ1P
 n8zJP8WZNeRw4Q9TtqVK09+oLirMUSpIDd6KdZ1VgRxOK2re7tjDvkVuYsSrsiJ+1iJNEnp9
 iK0ok0DlJpSCe6KhkxpaTdeoWMXdKuJWec0NIqoAd54ZgBPnr+UPxTixgPq/p6Q=
Message-ID: <22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com>
Date: Wed, 27 Feb 2019 19:45:11 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>
Content-Type: multipart/signed; micalg=pgp-sha512;
 protocol="application/pgp-signature";
 boundary="Sqns5SoT337PTwo6yN9u8oHfTqz0l3YrV"
Precedence: bulk

This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--Sqns5SoT337PTwo6yN9u8oHfTqz0l3YrV
Content-Type: multipart/mixed; boundary="z9MS5dP3dqLXGD7Yr71hSc42PLYq7ztTr";
 protected-headers="v1"
From: Joe Conway <mail@joeconway.com>
To: Michael Paquier <michael@paquier.xyz>,
 Postgres hackers <pgsql-hackers@lists.postgresql.org>
Message-ID: <22d70be7-9334-bfdc-a071-2efca3c52b98@joeconway.com>
Subject: Re: get_controlfile() can leak fds in the backend
References: <20190227074728.GA15710@paquier.xyz>
 <748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>
In-Reply-To: <748d7697-a25f-029d-e607-dbe4546a1b38@joeconway.com>

--z9MS5dP3dqLXGD7Yr71hSc42PLYq7ztTr
Content-Type: multipart/mixed;
 boundary="------------9E7FA5345E72704F05A918EC"
Content-Language: en-US

This is a multi-part message in MIME format.
--------------9E7FA5345E72704F05A918EC
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

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 issu=
e
> 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

--=20
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

--------------9E7FA5345E72704F05A918EC
Content-Type: text/x-patch;
 name="controldata_utils-fdfix-01.diff"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment;
 filename="controldata_utils-fdfix-01.diff"

diff --git a/src/common/controldata_utils.c b/src/common/controldata_util=
s.c
index 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
 =20
  /*
   * get_controlfile(char *DataDir, const char *progname, bool *crc_ok_p)=

*************** get_controlfile(const char *DataDir, con
*** 51,63 ****
  	ControlFile =3D palloc(sizeof(ControlFileData));
  	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);=

 =20
- 	if ((fd =3D open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) =3D=3D -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 =3D palloc(sizeof(ControlFileData));
  	snprintf(ControlFilePath, MAXPGPATH, "%s/global/pg_control", DataDir);=

 =20
  #ifndef FRONTEND
+ 	if ((fd =3D OpenTransientFile(ControlFilePath, O_RDONLY | PG_BINARY)) =
=3D=3D -1)
  		ereport(ERROR,
  				(errcode_for_file_access(),
  				 errmsg("could not open file \"%s\" for reading: %m",
  						ControlFilePath)));
  #else
+ 	if ((fd =3D open(ControlFilePath, O_RDONLY | PG_BINARY, 0)) =3D=3D -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
  	}
 =20
+ #ifndef FRONTEND
+ 	CloseTransientFile(fd);
+ #else
  	close(fd);
+ #endif
 =20
  	/* Check the CRC. */
  	INIT_CRC32C(crc);

--------------9E7FA5345E72704F05A918EC--

--z9MS5dP3dqLXGD7Yr71hSc42PLYq7ztTr--

--Sqns5SoT337PTwo6yN9u8oHfTqz0l3YrV
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEg14x9eymXoJyHrH+N/L3QzX32GUFAlx3LxcACgkQN/L3QzX3
2GWsAw/+LOx63SXKvHa5lG6jaeALbHl4m8Hq3T4JSye7bRf6uV8CK2eW1uxip3Tr
RebbJ+fuKXBHnuLMbtd7WfoLqeDpi2jL/j01rxNNCG5zW3+iVt9sXy7tIbmQi1Hp
5Tiu9TXVtsAIrAkVVuO8Fxh4KPUKiYi5qADoiLsq3AwXn4OAzeJvoPl4X4QJFZjn
ryyQ7PUyBwEc0jNVbP4sggp8NtZDnty/Kc2G8yNEx44gj62c/NONgh9Mg010Vf1c
PO9OZ0XZECBVmNdjHU8Qrzy5UzyHF3lGI9LPMsugmcOWhq9OG7ca1dpLymnGn2QL
OTVrmOWnTcDGzFhvOExlHZI+0Hnm5tF8KctslbPv7YIMir1DT5BIrDcYIXmAQNOb
QOWfmDveMh41kkb1Adl2lGnYeDFoqlAF1cREn4B1pGxJckcmunDOIJVPNSZFexH7
yaxg3W0iSMUxsriBVqsfM/M5Tb1ZyDdLbY5MNwsLzQfEXMWiV9y0BnCOBYrno2Dw
MX0VnCligzmjkV7kGl69ZHBzFtANvRPsHNiIV9H8inxcAA6KcdD/rnzlwwqLpOAE
p2Y5vln1ETE47zgZTwIsL6y/uelaNpu2rS4e5PhD7JNnly/mt0N7f+C992wEWzJf
YNZVTbMd/88Mb5Qb15FqpLAVabE0pCUSDudtRO1jPLyJq59pgtY=
=Yrtn
-----END PGP SIGNATURE-----

--Sqns5SoT337PTwo6yN9u8oHfTqz0l3YrV--