Fix assorted inconsistencies in our calls of readlink().

Ensure that we null-terminate the result string (one place in pg_rewind).
Be paranoid about out-of-range results from readlink() (should not happen,
but there is no good reason for some call sites to be careful about it and
others not).  Consistently use the whole buffer, not sometimes one byte
less.  Ensure we emit an appropriate errcode() in all cases.  Spell the
error messages the same way.

The only serious bug here is the missing null-termination in pg_rewind,
which is new code, so no need for a back-patch.

Abhijit Menon-Sen and Tom Lane

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index fa29624667ef5f933f8f1036b58e876c976b78b8..1e86e4c57b64a65752cb9e0fdef9340c3bd8a440 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1028,7 +1028,8 @@ sendDir(char *path, int basepathlen, bool sizeonly, List *tablespaces,
                                                                pathbuf)));
                        if (rllen >= sizeof(linkpath))
                                ereport(ERROR,
-                                               (errmsg("symbolic link \"%s\" target is too long",
+                                               (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                                errmsg("symbolic link \"%s\" target is too long",
                                                                pathbuf)));
                        linkpath[rllen] = '\0';
 

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 68d43c66b6fa28cde5afe8f345e282e8579d6f64..78aade98d1d67e7fcaee59c2a0ecf2b23f8a9ce5 100644 (file)
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -2517,18 +2517,17 @@ walkdir(char *path, void (*action) (char *fname, bool isdir))
                        int                     len;
                        struct stat lst;
 
-                       len = readlink(subpath, linkpath, sizeof(linkpath) - 1);
+                       len = readlink(subpath, linkpath, sizeof(linkpath));
                        if (len < 0)
                                ereport(ERROR,
                                                (errcode_for_file_access(),
                                                 errmsg("could not read symbolic link \"%s\": %m",
                                                                subpath)));
-
-                       if (len >= sizeof(linkpath) - 1)
+                       if (len >= sizeof(linkpath))
                                ereport(ERROR,
-                                               (errmsg("symbolic link \"%s\" target is too long",
+                                               (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                                errmsg("symbolic link \"%s\" target is too long",
                                                                subpath)));
-
                        linkpath[len] = '\0';
 
                        if (lstat(linkpath, &lst) == 0)

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index de68cdddf1da38cfb169676d8bb2877de704e413..c0495d955ce6f8da000eb1fe6fd4c4868c75cc26 100644 (file)
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -374,11 +374,13 @@ pg_tablespace_location(PG_FUNCTION_ARGS)
        rllen = readlink(sourcepath, targetpath, sizeof(targetpath));
        if (rllen < 0)
                ereport(ERROR,
-                               (errmsg("could not read symbolic link \"%s\": %m",
+                               (errcode_for_file_access(),
+                                errmsg("could not read symbolic link \"%s\": %m",
                                                sourcepath)));
-       else if (rllen >= sizeof(targetpath))
+       if (rllen >= sizeof(targetpath))
                ereport(ERROR,
-                               (errmsg("symbolic link \"%s\" target is too long",
+                               (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                                errmsg("symbolic link \"%s\" target is too long",
                                                sourcepath)));
        targetpath[rllen] = '\0';
 

diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 9e317a2c7cbc3f666fc525e9088b0a91435db141..c92744ca77245924462d4b87a6357821f52d7e3d 100644 (file)
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -112,19 +112,16 @@ recurse_dir(const char *datadir, const char *parentpath,
                {
 #if defined(HAVE_READLINK) || defined(WIN32)
                        char            link_target[MAXPGPATH];
-                       ssize_t         len;
+                       int                     len;
 
-                       len = readlink(fullpath, link_target, sizeof(link_target) - 1);
-                       if (len == -1)
-                               pg_fatal("readlink() failed on \"%s\": %s\n",
+                       len = readlink(fullpath, link_target, sizeof(link_target));
+                       if (len < 0)
+                               pg_fatal("could not read symbolic link \"%s\": %s\n",
                                                 fullpath, strerror(errno));
-
-                       if (len == sizeof(link_target) - 1)
-                       {
-                               /* path was truncated */
-                               pg_fatal("symbolic link \"%s\" target path too long\n",
+                       if (len >= sizeof(link_target))
+                               pg_fatal("symbolic link \"%s\" target is too long\n",
                                                 fullpath);
-                       }
+                       link_target[len] = '\0';
 
                        callback(path, FILE_TYPE_SYMLINK, 0, link_target);