From 17901b013709a0e46056b20001ee620016a286c6 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sat, 13 Oct 2012 12:48:14 +0300 Subject: [PATCH] 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. --- src/bin/initdb/initdb.c | 12 +++--- src/bin/pg_ctl/pg_ctl.c | 88 +++++++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 37 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 5f6e1d6e077..af8dc1b56d8 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -375,6 +375,7 @@ readfile(char *path) int maxlength = 0, linelen = 0; int nlines = 0; + int n; char **result; char *buffer; int c; @@ -415,16 +416,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 4f0daf5401d..7f1932e5f6f 100644 --- 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 #include #include #include @@ -301,50 +302,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; } -- 2.39.5