From 1e7d5d155257bbce1c35c9f0ce2e0fa57b1c7f51 Mon Sep 17 00:00:00 2001 From: Daniel Gustafsson Date: Tue, 23 Feb 2021 16:48:38 +0100 Subject: [PATCH v5] Check version of target cluster binaries This expands the binary validation in pg_upgrade with a version check per binary to ensure the health of the target cluster. In order to reduce duplication, validate_exec is exported from port.h and the local copy in pg_upgrade is removed. --- src/bin/pg_upgrade/exec.c | 85 +++++++++++++++------------------------ src/common/exec.c | 3 +- src/include/port.h | 1 + 3 files changed, 35 insertions(+), 54 deletions(-) diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c index 43a4565c2e..71471143e3 100644 --- a/src/bin/pg_upgrade/exec.c +++ b/src/bin/pg_upgrade/exec.c @@ -16,7 +16,7 @@ static void check_data_dir(ClusterInfo *cluster); static void check_bin_dir(ClusterInfo *cluster); static void get_bin_version(ClusterInfo *cluster); -static void validate_exec(const char *dir, const char *cmdName); +static void find_exec(const char *dir, const char *program); #ifdef WIN32 static int win32_check_directory_write_permissions(void); @@ -375,9 +375,9 @@ check_bin_dir(ClusterInfo *cluster) report_status(PG_FATAL, "\"%s\" is not a directory\n", cluster->bindir); - validate_exec(cluster->bindir, "postgres"); - validate_exec(cluster->bindir, "pg_controldata"); - validate_exec(cluster->bindir, "pg_ctl"); + find_exec(cluster->bindir, "postgres"); + find_exec(cluster->bindir, "pg_controldata"); + find_exec(cluster->bindir, "pg_ctl"); /* * Fetch the binary version after checking for the existence of pg_ctl. @@ -388,9 +388,9 @@ check_bin_dir(ClusterInfo *cluster) /* pg_resetxlog has been renamed to pg_resetwal in version 10 */ if (GET_MAJOR_VERSION(cluster->bin_version) <= 906) - validate_exec(cluster->bindir, "pg_resetxlog"); + find_exec(cluster->bindir, "pg_resetxlog"); else - validate_exec(cluster->bindir, "pg_resetwal"); + find_exec(cluster->bindir, "pg_resetwal"); if (cluster == &new_cluster) { @@ -399,63 +399,44 @@ check_bin_dir(ClusterInfo *cluster) * pg_dumpall are used to dump the old cluster, but must be of the * target version. */ - validate_exec(cluster->bindir, "initdb"); - validate_exec(cluster->bindir, "pg_dump"); - validate_exec(cluster->bindir, "pg_dumpall"); - validate_exec(cluster->bindir, "pg_restore"); - validate_exec(cluster->bindir, "psql"); - validate_exec(cluster->bindir, "vacuumdb"); + find_exec(cluster->bindir, "initdb"); + find_exec(cluster->bindir, "pg_dump"); + find_exec(cluster->bindir, "pg_dumpall"); + find_exec(cluster->bindir, "pg_restore"); + find_exec(cluster->bindir, "psql"); + find_exec(cluster->bindir, "vacuumdb"); } } - -/* - * validate_exec() - * - * validate "path" as an executable file - */ static void -validate_exec(const char *dir, const char *cmdName) +find_exec(const char *dir, const char *program) { - char path[MAXPGPATH]; - struct stat buf; + char path[MAXPGPATH]; + char line[MAXPGPATH]; + char cmd[MAXPGPATH]; + char versionstr[128]; + int ret; - snprintf(path, sizeof(path), "%s/%s", dir, cmdName); + snprintf(path, sizeof(path), "%s/%s%s", dir, program, EXE); -#ifdef WIN32 - /* Windows requires a .exe suffix for stat() */ - if (strlen(path) <= strlen(EXE_EXT) || - pg_strcasecmp(path + strlen(path) - strlen(EXE_EXT), EXE_EXT) != 0) - strlcat(path, EXE_EXT, sizeof(path)); -#endif + ret = validate_exec(path); - /* - * Ensure that the file exists and is a regular file. - */ - if (stat(path, &buf) < 0) - pg_fatal("check for \"%s\" failed: %s\n", - path, strerror(errno)); - else if (!S_ISREG(buf.st_mode)) + if (ret == -1) pg_fatal("check for \"%s\" failed: not a regular file\n", path); - - /* - * Ensure that the file is both executable and readable (required for - * dynamic loading). - */ -#ifndef WIN32 - if (access(path, R_OK) != 0) -#else - if ((buf.st_mode & S_IRUSR) == 0) -#endif - pg_fatal("check for \"%s\" failed: cannot read file (permission denied)\n", + else if (ret == -2) + pg_fatal("check for \"%s\" failed: cannot execute (permission denied)\n", path); -#ifndef WIN32 - if (access(path, X_OK) != 0) -#else - if ((buf.st_mode & S_IXUSR) == 0) -#endif - pg_fatal("check for \"%s\" failed: cannot execute (permission denied)\n", + snprintf(cmd, sizeof(cmd), "\"%s\" -V", path); + + if (!pipe_read_line(cmd, line, sizeof(line))) + pg_fatal("check for \"%s\" failed: cannot execute\n", path); + + snprintf(versionstr, sizeof(versionstr), "%s (PostgreSQL) " PG_VERSION "\n", program); + + if (strcmp(line, versionstr) != 0) + pg_fatal("check for \"%s\" failed: incorrect version (\"%s\" vs \"%s\")\n", + path, line, versionstr); } diff --git a/src/common/exec.c b/src/common/exec.c index 6cbc820042..66c3aa6acc 100644 --- a/src/common/exec.c +++ b/src/common/exec.c @@ -49,7 +49,6 @@ #define getcwd(cwd,len) GetCurrentDirectory(len, cwd) #endif -static int validate_exec(const char *path); static int resolve_symlinks(char *path); #ifdef WIN32 @@ -63,7 +62,7 @@ static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser); * -1 if the regular file "path" does not exist or cannot be executed. * -2 if the file is otherwise valid but cannot be read. */ -static int +int validate_exec(const char *path) { struct stat buf; diff --git a/src/include/port.h b/src/include/port.h index 6486db9fdd..227ef4b148 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -125,6 +125,7 @@ extern void pgfnames_cleanup(char **filenames); extern void set_pglocale_pgservice(const char *argv0, const char *app); /* Portable way to find and execute binaries (in exec.c) */ +extern int validate_exec(const char *path); extern int find_my_exec(const char *argv0, char *retpath); extern int find_other_exec(const char *argv0, const char *target, const char *versionstr, char *retpath); -- 2.21.1 (Apple Git-122.3)