Re: To Bruce Momjian 2014-07-11 <20140711093923.ga3...@msg.df7cb.de>
> Re: Bruce Momjian 2014-07-08 <20140708202114.gd9...@momjian.us>
> > > > > I believe pg_upgrade itself still needs a fix. While it's not a
> > > > > security problem to put the socket in $CWD while upgrading (it is
> > > > > using -c unix_socket_permissions=0700), this behavior is pretty
> > > > > unexpected, and does fail if your $CWD is > 107 bytes.
> > > > >
> > > > > In f545d233ebce6971b6f9847680e48b679e707d22 Peter fixed the pg_ctl
> > > > > perl tests to avoid that problem, so imho it would make even more
> > > > > sense to fix pg_upgrade which could also fail in production.
> > > >
> > > > +1. Does writing that patch interest you?
> > >
> > > I'll give it a try once I've finished this CF review.
> >
> > OK. Let me know if you need help.
>
> Here's the patch. Proposed commit message:
>
> Create pg_upgrade sockets in temp directories
>
> pg_upgrade used to use the current directory for UNIX sockets to
> access the old/new cluster. This fails when the current path is
> > 107 bytes. Fix by reusing the tempdir code from pg_regress
> introduced in be76a6d39e2832d4b88c0e1cc381aa44a7f86881. For cleanup,
> we need to remember up to two directories.
Uh... now really.
Christoph
--
c...@df7cb.de | http://www.df7cb.de/
diff --git a/contrib/pg_upgrade/option.c b/contrib/pg_upgrade/option.c
index b81010a..121a3d4 100644
--- a/contrib/pg_upgrade/option.c
+++ b/contrib/pg_upgrade/option.c
@@ -14,6 +14,7 @@
#include "pg_upgrade.h"
+#include <signal.h>
#include <time.h>
#include <sys/types.h>
#ifdef WIN32
@@ -26,6 +27,13 @@ static void check_required_directory(char **dirpath, char **configpath,
char *envVarName, char *cmdLineOption, char *description);
#define FIX_DEFAULT_READ_ONLY "-c default_transaction_read_only=false"
+#ifdef HAVE_UNIX_SOCKETS
+/* make_temp_sockdir() is invoked at most twice from pg_upgrade.c via get_sock_dir() */
+#define MAX_TEMPDIRS 2
+static int n_tempdirs = 0; /* actual number of directories created */
+static const char *temp_sockdir[MAX_TEMPDIRS];
+#endif
+
UserOpts user_opts;
@@ -396,6 +404,86 @@ adjust_data_dir(ClusterInfo *cluster)
check_ok();
}
+#ifdef HAVE_UNIX_SOCKETS
+/*
+ * Remove the socket temporary directories. pg_ctl waits for postmaster
+ * shutdown, so we expect the directory to be empty, unless we are interrupted
+ * by a signal, in which case the postmaster will clean up the sockets, but
+ * there's a race condition with us removing the directory. Ignore errors;
+ * leaking a (usually empty) temporary directory is unimportant. This can run
+ * from a signal handler. The code is not acceptable in a Windows signal
+ * handler (see initdb.c:trapsig()), but Windows is not a HAVE_UNIX_SOCKETS
+ * platform.
+ */
+static void remove_temp(void)
+{
+ int i;
+
+ for (i = 0; i < n_tempdirs; i++)
+ {
+ Assert(temp_sockdir[i]);
+ rmdir(temp_sockdir[i]);
+ }
+}
+
+/*
+ * Signal handler that calls remove_temp() and reraises the signal.
+ */
+static void
+signal_remove_temp(int signum)
+{
+ remove_temp();
+
+ pqsignal(signum, SIG_DFL);
+ raise(signum);
+}
+
+/*
+ * Create a temporary directory suitable for the server's Unix-domain socket.
+ * The directory will have mode 0700 or stricter, so no other OS user can open
+ * our socket to exploit it independently from the auth method used. Most
+ * systems constrain the length of socket paths well below _POSIX_PATH_MAX, so
+ * we place the directory under /tmp rather than relative to the possibly-deep
+ * current working directory.
+ *
+ * Using a temporary directory so no connections arrive other than what
+ * pg_upgrade initiate itself. Compared to using the compiled-in
+ * DEFAULT_PGSOCKET_DIR, this also permits pg_upgrade to work in builds that
+ * relocate it to a directory not writable to the cluster owner.
+ */
+static const char *
+make_temp_sockdir(void)
+{
+ char *template = strdup("/tmp/pg_upgrade-XXXXXX");
+
+ Assert(n_tempdirs < MAX_TEMPDIRS);
+ temp_sockdir[n_tempdirs] = mkdtemp(template);
+ if (temp_sockdir[n_tempdirs] == NULL)
+ {
+ fprintf(stderr, _("%s: could not create directory \"%s\": %s\n"),
+ os_info.progname, template, strerror(errno));
+ exit(2);
+ }
+
+ /*
+ * Remove the directories during clean exit. Register the handler only
+ * once, though.
+ */
+ if (n_tempdirs == 0)
+ atexit(remove_temp);
+
+ /*
+ * Remove the directory before dying to the usual signals. Omit SIGQUIT,
+ * preserving it as a quick, untidy exit.
+ */
+ pqsignal(SIGHUP, signal_remove_temp);
+ pqsignal(SIGINT, signal_remove_temp);
+ pqsignal(SIGPIPE, signal_remove_temp);
+ pqsignal(SIGTERM, signal_remove_temp);
+
+ return temp_sockdir[n_tempdirs++];
+}
+#endif /* HAVE_UNIX_SOCKETS */
/*
* get_sock_dir
@@ -418,10 +506,8 @@ get_sock_dir(ClusterInfo *cluster, bool live_check)
{
if (!live_check)
{
- /* Use the current directory for the socket */
- cluster->sockdir = pg_malloc(MAXPGPATH);
- if (!getcwd(cluster->sockdir, MAXPGPATH))
- pg_fatal("cannot find current directory\n");
+ /* Use a temporary directory for the socket */
+ cluster->sockdir = make_temp_sockdir();
}
else
{
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
index 0410b02..c58f0f0 100644
--- a/contrib/pg_upgrade/pg_upgrade.h
+++ b/contrib/pg_upgrade/pg_upgrade.h
@@ -250,7 +250,7 @@ typedef struct
char *bindir; /* pathname for cluster's executable directory */
char *pgopts; /* options to pass to the server, like pg_ctl
* -o */
- char *sockdir; /* directory for Unix Domain socket, if any */
+ char const *sockdir; /* directory for Unix Domain socket, if any */
unsigned short port; /* port number where postmaster is waiting */
uint32 major_version; /* PG_VERSION of cluster */
char major_version_str[64]; /* string PG_VERSION of cluster */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
