Be more conservative about removing tablespace "symlinks".

Don't apply rmtree(), which will gleefully remove an entire subtree,
and don't even apply unlink() unless it's symlink or a directory,
the only things that we expect to find.

Amit Kapila, with minor tweaks by me, per extensive discussions
involving Andrew Dunstan, Fujii Masao, and Heikki Linnakangas,
at least some of whom also reviewed the code.

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 4e37ad3e21aa4c9dc2fd41f2c0b72a1d636e7ecf..7830b47c8d1cb27ff6c55a73e0e4d4f112a48144 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -38,6 +38,7 @@
 #include "catalog/catversion.h"
 #include "catalog/pg_control.h"
 #include "catalog/pg_database.h"
+#include "commands/tablespace.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "postmaster/bgwriter.h"
@@ -6074,7 +6075,6 @@ StartupXLOG(void)
        if (read_tablespace_map(&tablespaces))
        {
            ListCell   *lc;
-           struct stat st;
 
            foreach(lc, tablespaces)
            {
@@ -6085,27 +6085,9 @@ StartupXLOG(void)
 
                /*
                 * Remove the existing symlink if any and Create the symlink
-                * under PGDATA.  We need to use rmtree instead of rmdir as
-                * the link location might contain directories or files
-                * corresponding to the actual path. Some tar utilities do
-                * things that way while extracting symlinks.
+                * under PGDATA.
                 */
-               if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
-               {
-                   if (!rmtree(linkloc, true))
-                       ereport(ERROR,
-                               (errcode_for_file_access(),
-                             errmsg("could not remove directory \"%s\": %m",
-                                    linkloc)));
-               }
-               else
-               {
-                   if (unlink(linkloc) < 0 && errno != ENOENT)
-                       ereport(ERROR,
-                               (errcode_for_file_access(),
-                         errmsg("could not remove symbolic link \"%s\": %m",
-                                linkloc)));
-               }
+               remove_tablespace_symlink(linkloc);
 
                if (symlink(ti->path, linkloc) < 0)
                    ereport(ERROR,

diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4ec1affbfb16f81f004156c8e413f58d32cdb9fb..ff0d904b7a865b32585a81d7790dcd79d3004ae4 100644 (file)
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -627,31 +627,9 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
 
    /*
     * In recovery, remove old symlink, in case it points to the wrong place.
-    *
-    * On Windows, junction points act like directories so we must be able to
-    * apply rmdir; in general it seems best to make this code work like the
-    * symlink removal code in destroy_tablespace_directories, except that
-    * failure to remove is always an ERROR.
     */
    if (InRecovery)
-   {
-       if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
-       {
-           if (rmdir(linkloc) < 0)
-               ereport(ERROR,
-                       (errcode_for_file_access(),
-                        errmsg("could not remove directory \"%s\": %m",
-                               linkloc)));
-       }
-       else
-       {
-           if (unlink(linkloc) < 0 && errno != ENOENT)
-               ereport(ERROR,
-                       (errcode_for_file_access(),
-                        errmsg("could not remove symbolic link \"%s\": %m",
-                               linkloc)));
-       }
-   }
+       remove_tablespace_symlink(linkloc);
 
    /*
     * Create the symlink under PGDATA
@@ -802,7 +780,8 @@ remove_symlink:
                     errmsg("could not remove directory \"%s\": %m",
                            linkloc)));
    }
-   else
+#ifdef S_ISLNK
+   else if (S_ISLNK(st.st_mode))
    {
        if (unlink(linkloc) < 0)
        {
@@ -814,6 +793,15 @@ remove_symlink:
                            linkloc)));
        }
    }
+#endif
+   else
+   {
+       /* Refuse to remove anything that's not a directory or symlink */
+       ereport(redo ? LOG : ERROR,
+               (ERRCODE_SYSTEM_ERROR,
+                errmsg("not a directory or symbolic link: \"%s\"",
+                       linkloc)));
+   }
 
    pfree(linkloc_with_version_dir);
    pfree(linkloc);
@@ -848,6 +836,59 @@ directory_is_empty(const char *path)
    return true;
 }
 
+/*
+ * remove_tablespace_symlink
+ *
+ * This function removes symlinks in pg_tblspc.  On Windows, junction points
+ * act like directories so we must be able to apply rmdir.  This function
+ * works like the symlink removal code in destroy_tablespace_directories,
+ * except that failure to remove is always an ERROR.  But if the file doesn't
+ * exist at all, that's OK.
+ */
+void
+remove_tablespace_symlink(const char *linkloc)
+{
+   struct stat st;
+
+   if (lstat(linkloc, &st) != 0)
+   {
+       if (errno == ENOENT)
+           return;
+       ereport(ERROR,
+               (errcode_for_file_access(),
+                errmsg("could not stat \"%s\": %m", linkloc)));
+   }
+
+   if (S_ISDIR(st.st_mode))
+   {
+       /*
+        * This will fail if the directory isn't empty, but not
+        * if it's a junction point.
+        */
+       if (rmdir(linkloc) < 0)
+           ereport(ERROR,
+                   (errcode_for_file_access(),
+                    errmsg("could not remove directory \"%s\": %m",
+                           linkloc)));
+   }
+#ifdef S_ISLNK
+   else if (S_ISLNK(st.st_mode))
+   {
+       if (unlink(linkloc) < 0 && errno != ENOENT)
+           ereport(ERROR,
+                   (errcode_for_file_access(),
+                       errmsg("could not remove symbolic link \"%s\": %m",
+                           linkloc)));
+   }
+#endif
+   else
+   {
+       /* Refuse to remove anything that's not a directory or symlink */
+       ereport(ERROR,
+               (errmsg("not a directory or symbolic link: \"%s\"",
+                       linkloc)));
+   }
+}
 
 /*
  * Rename a tablespace

diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h
index 86b0477335b50023eaee83bc23b09cdc40504ac8..6b928a58a0120207873267d767a6149b11fdda11 100644 (file)
--- a/src/include/commands/tablespace.h
+++ b/src/include/commands/tablespace.h
@@ -56,6 +56,7 @@ extern Oid    get_tablespace_oid(const char *tablespacename, bool missing_ok);
 extern char *get_tablespace_name(Oid spc_oid);
 
 extern bool directory_is_empty(const char *path);
+extern void remove_tablespace_symlink(const char *linkloc);
 
 extern void tblspc_redo(XLogReaderState *rptr);
 extern void tblspc_desc(StringInfo buf, XLogReaderState *rptr);