Fix race condition in pg_ctl reading postmaster.pid.

If postmaster changed postmaster.pid while pg_ctl was reading it, pg_ctl
could overrun the buffer it allocated for the file. Fix by reading the
whole file to memory with one read() call.

initdb contains an identical copy of the readfile() function, but the files
that initdb reads are static, not modified concurrently. Nevertheless, add
a simple bounds-check there, if only to silence static analysis tools.

Per report from Dave Vitek. Backpatch to all supported branches.

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index d5101fad2fcda5c3d3ebf58edf9fec764ffee6a5..f3e1d90dc41d5050e980238a4147e5b65aab3542 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -368,6 +368,7 @@ readfile(const char *path)
        int                     maxlength = 1,
                                linelen = 0;
        int                     nlines = 0;
+       int                     n;
        char      **result;
        char       *buffer;
        int                     c;
@@ -405,13 +406,13 @@ readfile(const char *path)
 
        /* now reprocess the file and store the lines */
        rewind(infile);
-       nlines = 0;
-       while (fgets(buffer, maxlength + 1, infile) != NULL)
-               result[nlines++] = xstrdup(buffer);
+       n = 0;
+       while (fgets(buffer, maxlength + 1, infile) != NULL && n < nlines)
+               result[n++] = xstrdup(buffer);
 
        fclose(infile);
        free(buffer);
-       result[nlines] = NULL;
+       result[n] = NULL;
 
        return result;
 }

diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index ffd014ea53eb7a4986bb269590f726a3685fa5d0..336f3df3b52c335e4d97b25a27b1d25e8a811c3a 100644 (file)
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -20,6 +20,7 @@
 #include "postgres_fe.h"
 #include "libpq-fe.h"
 
+#include <fcntl.h>
 #include <locale.h>
 #include <signal.h>
 #include <sys/types.h>
@@ -297,50 +298,77 @@ get_pgpid(void)
 static char **
 readfile(const char *path)
 {
-       FILE       *infile;
-       int                     maxlength = 1,
-                               linelen = 0;
-       int                     nlines = 0;
+       int                     fd;
+       int                     nlines;
        char      **result;
        char       *buffer;
-       int                     c;
+       char       *linebegin;
+       int                     i;
+       int                     n;
+       int                     len;
+       struct stat     statbuf;
 
-       if ((infile = fopen(path, "r")) == NULL)
+       /*
+        * Slurp the file into memory.
+        *
+        * The file can change concurrently, so we read the whole file into memory
+        * with a single read() call. That's not guaranteed to get an atomic
+        * snapshot, but in practice, for a small file, it's close enough for the
+        * current use.
+        */
+       fd = open(path, O_RDONLY | PG_BINARY, 0);
+       if (fd < 0)
                return NULL;
+       if (fstat(fd, &statbuf) < 0)
+               return NULL;
+       if (statbuf.st_size == 0)
+       {
+               /* empty file */
+               result = (char **) pg_malloc(sizeof(char *));
+               *result = NULL;
+               return result;
+       }
+       buffer = pg_malloc(statbuf.st_size + 1);
 
-       /* pass over the file twice - the first time to size the result */
+       len = read(fd, buffer, statbuf.st_size + 1);
+       close(fd);
+       if (len != statbuf.st_size)
+       {
+               /* oops, the file size changed between fstat and read */
+               free(buffer);
+               return NULL;
+       }
 
-       while ((c = fgetc(infile)) != EOF)
+       /* count newlines */
+       nlines = 0;
+       for (i = 0; i < len - 1; i++)
        {
-               linelen++;
-               if (c == '\n')
-               {
+               if (buffer[i] == '\n')
                        nlines++;
-                       if (linelen > maxlength)
-                               maxlength = linelen;
-                       linelen = 0;
-               }
        }
+       nlines++; /* account for the last line */
 
-       /* handle last line without a terminating newline (yuck) */
-       if (linelen)
-               nlines++;
-       if (linelen > maxlength)
-               maxlength = linelen;
-
-       /* set up the result and the line buffer */
+       /* set up the result buffer */
        result = (char **) pg_malloc((nlines + 1) * sizeof(char *));
-       buffer = (char *) pg_malloc(maxlength + 1);
 
-       /* now reprocess the file and store the lines */
-       rewind(infile);
-       nlines = 0;
-       while (fgets(buffer, maxlength + 1, infile) != NULL)
-               result[nlines++] = xstrdup(buffer);
+       /* now split the buffer into lines */
+       linebegin = buffer;
+       n = 0;
+       for (i = 0; i < len; i++)
+       {
+               if (buffer[i] == '\n' || i == len - 1)
+               {
+                       int             slen = &buffer[i] - linebegin + 1;
+                       char   *linebuf = pg_malloc(slen + 1);
+                       memcpy(linebuf, linebegin, slen);
+                       linebuf[slen] = '\0';
+                       result[n++] = linebuf;
+                       linebegin = &buffer[i + 1];
+               }
+       }
+       result[n] = NULL;
 
-       fclose(infile);
        free(buffer);
-       result[nlines] = NULL;
 
        return result;
 }