]> granicus.if.org Git - postgresql/commitdiff
Fix multiple bugs in tablespace symlink removal.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 8 Apr 2016 16:31:42 +0000 (12:31 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 8 Apr 2016 16:31:53 +0000 (12:31 -0400)
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

index 1ff57285e5949b6a8c80c98c2415ebe66c830dc9..7902d433d552e4b627f975f74167b90c19d95d1d 100644 (file)
@@ -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)));
        }
 }