get_controlfile() can leak fds in the backend

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Joe Conway <mail(at)joeconway(dot)com>
Subject: get_controlfile() can leak fds in the backend
Date: 2019-02-27 07:47:28
Message-ID: 20190227074728.GA15710@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,
(CC-ing Joe as of dc7d70e)

I was just looking at the offline checksum patch, and noticed some
sloppy coding in controldata_utils.c. The control file gets opened in
get_controlfile(), and if it generates an error then the file
descriptor remains open. As basic code rule in the backend we should
only use BasicOpenFile() when opening files, so I think that the issue
should be fixed as attached, even if this requires including fd.h for
the backend compilation which is kind of ugly.

Another possibility would be to just close the file descriptor before
any error, saving appropriately errno before issuing any %m portion,
but that still does not respect the backend policy regarding open().

We also do not have a wait event for the read() call, maybe we should
have one, but knowing that this gets called only for the SQL-level
functions accessing the control file, I don't really think that's
worth it.

Thoughts?
--
Michael

Attachment Content-Type Size
controlfile-leak.patch text/x-diff 1.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2019-02-27 08:14:59 Re: psql show URL with help
Previous Message Laurenz Albe 2019-02-27 07:35:21 Re: Remove Deprecated Exclusive Backup Mode