From 93c301fc4ff7d4f06bff98fea8db47ce67f28155 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 8 Apr 2016 12:31:42 -0400 Subject: [PATCH] Fix multiple bugs in tablespace symlink removal. Don't try to examine S_ISLNK(st.st_mode) after a failed lstat(). It's undefined. Also, if the lstat() reported ENOENT, we do not wish that to be a hard error, but the code might nonetheless treat it as one (giving an entirely misleading error message, too) depending on luck-of-the-draw as to what S_ISLNK() returned. Don't throw error for ENOENT from rmdir(), either. (We're not really expecting ENOENT because we just stat'd the file successfully; but if we're going to allow ENOENT in the symlink code path, surely the directory code path should too.) Generate an appropriate errcode for its-the-wrong-type-of-file complaints. (ERRCODE_SYSTEM_ERROR doesn't seem appropriate, and failing to write errcode() around it certainly doesn't work, and not writing an errcode at all is not per project policy.) Valgrind noticed the undefined S_ISLNK result; the other problems emerged while reading the code in the area. All of this appears to have been introduced in 8f15f74a44f68f9c. Back-patch to 9.5 where that commit appeared. --- src/backend/commands/tablespace.c | 32 ++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index 1ff57285e5..7902d433d5 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -773,13 +773,26 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) remove_symlink: linkloc = pstrdup(linkloc_with_version_dir); get_parent_directory(linkloc); - if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode)) + if (lstat(linkloc, &st) < 0) + { + int saved_errno = errno; + + ereport(redo ? LOG : (saved_errno == ENOENT ? WARNING : ERROR), + (errcode_for_file_access(), + errmsg("could not stat file \"%s\": %m", + linkloc))); + } + else if (S_ISDIR(st.st_mode)) { if (rmdir(linkloc) < 0) - ereport(redo ? LOG : ERROR, + { + int saved_errno = errno; + + ereport(redo ? LOG : (saved_errno == ENOENT ? WARNING : ERROR), (errcode_for_file_access(), errmsg("could not remove directory \"%s\": %m", linkloc))); + } } #ifdef S_ISLNK else if (S_ISLNK(st.st_mode)) @@ -799,7 +812,7 @@ remove_symlink: { /* Refuse to remove anything that's not a directory or symlink */ ereport(redo ? LOG : ERROR, - (ERRCODE_SYSTEM_ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("\"%s\" is not a directory or symbolic link", linkloc))); } @@ -851,7 +864,7 @@ remove_tablespace_symlink(const char *linkloc) { struct stat st; - if (lstat(linkloc, &st) != 0) + if (lstat(linkloc, &st) < 0) { if (errno == ENOENT) return; @@ -863,10 +876,10 @@ remove_tablespace_symlink(const char *linkloc) if (S_ISDIR(st.st_mode)) { /* - * This will fail if the directory isn't empty, but not - * if it's a junction point. + * This will fail if the directory isn't empty, but not if it's a + * junction point. */ - if (rmdir(linkloc) < 0) + if (rmdir(linkloc) < 0 && errno != ENOENT) ereport(ERROR, (errcode_for_file_access(), errmsg("could not remove directory \"%s\": %m", @@ -878,7 +891,7 @@ remove_tablespace_symlink(const char *linkloc) if (unlink(linkloc) < 0 && errno != ENOENT) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not remove symbolic link \"%s\": %m", + errmsg("could not remove symbolic link \"%s\": %m", linkloc))); } #endif @@ -886,7 +899,8 @@ remove_tablespace_symlink(const char *linkloc) { /* Refuse to remove anything that's not a directory or symlink */ ereport(ERROR, - (errmsg("\"%s\" is not a directory or symbolic link", + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("\"%s\" is not a directory or symbolic link", linkloc))); } }