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 9994d891d1f483611f5aa80e5d9e307fb0945ffe..a0b816112765f5258a215d862b47975dc89b1764 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -368,6 +368,7 @@ readfile(char *path)
        int                     maxlength = 0,
                                linelen = 0;
        int                     nlines = 0;
+       int                     n;
        char      **result;
        char       *buffer;
        int                     c;
@@ -408,16 +409,13 @@ readfile(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);
-               nlines++;
-       }
+       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 d8a11df4339ad793e2b65691f2a36451eb9403fd..8ec723bcdd2bb6a1e2f5eeaf4b65357515a4b595 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>
@@ -294,50 +295,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;
 }