8000 Fix race condition in pg_ctl reading postmaster.pid. · haying/postgres@6d934e4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6d934e4

Browse files
hlinnakahlinnaka
committed
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.
1 parent 55eaeef commit 6d934e4

File tree

2 files changed

+63
-37
lines changed

2 files changed

+63
-37
lines changed

‎src/bin/initdb/initdb.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ readfile(char *path)
368368
int maxlength = 0,
369369
linelen = 0;
370370
int nlines = 0;
371+
int n;
371372
char **result;
372373
char *buffer;
373374
int c;
@@ -408,16 +409,13 @@ readfile(char *path)
408409
/* now reprocess the file and store the lines */
409410

410411
rewind(infile);
411-
nlines = 0;
412-
while (fgets(buffer, maxlength + 1, infile) != NULL)
413-
{
414-
result[nlines] = xstrdup(buffer);
415-
nlines++;
8000 416-
}
412+
n = 0;
413+
while (fgets(buffer, maxlength + 1, infile) != NULL && n < nlines)
414+
result[n++] = xstrdup(buffer);
417415

418416
fclose(infile);
419417
free(buffer);
420-
result[nlines] = NULL;
418+
result[n] = NULL;
421419

422420
return result;
423421
}

‎src/bin/pg_ctl/pg_ctl.c

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "postgres_fe.h"
2121
#include "libpq-fe.h"
2222

23+
#include <fcntl.h>
2324
#include <locale.h>
24< 10000 code>25
#include <signal.h>
2526
#include <sys/types.h>
@@ -294,50 +295,77 @@ get_pgpid(void)
294295
static char **
295296
readfile(const char *path)
296297
{
297-
FILE *infile;
298-
int maxlength = 1,
299-
linelen = 0;
300-
int nlines = 0;
298+
int fd;
299+
int nlines;
301300
char **result;
302301
char *buffer;
303-
int c;
302+
char *linebegin;
303+
int i;
304+
int n;
305+
int len;
306+
struct stat statbuf;
304307

305-
if ((infile = fopen(path, "r")) == NULL)
308+
/*
309+
* Slurp the file into memory.
310+
*
311+
* The file can change concurrently, so we read the whole file into memory
312+
* with a single read() call. That's not guaranteed to get an atomic
313+
* snapshot, but in practice, for a small file, it's close enough for the
314+
* current use.
315+
*/
316+
fd = open(path, O_RDONLY | PG_BINARY, 0);
317+
if (fd < 0)
306318
return NULL;
319+
if (fstat(fd, &statbuf) < 0)
320+
return NULL;
321+
if (statbuf.st_size == 0)
322+
{
323+
/* empty file */
324+
result = (char **) pg_malloc(sizeof(char *));
325+
*result = NULL;
326+
return result;
327+
}
328+
buffer = pg_malloc(statbuf.st_size + 1);
307329

308-
/* pass over the file twice - the first time to size the result */< E29B /div>
330+
len = read(fd, buffer, statbuf.st_size + 1);
331+
close(fd);
332+
if (len != statbuf.st_size)
333+
{
334+
/* oops, the file size changed between fstat and read */
335+
free(buffer);
336+
return NULL;
337+
}
309338

310-
while ((c = fgetc(infile)) != EOF)
339+
/* count newlines */
340+
nlines = 0;
341+
for (i = 0; i < len - 1; i++)
311342
{
312-
linelen++;
313-
if (c == '\n')
314-
{
343+
if (buffer[i] == '\n')
315344
nlines++;
316-
if (linelen > maxlength)
317-
maxlength = linelen;
318-
linelen = 0;
319-
}
320345
}
346+
nlines++; /* account for the last line */
321347

322-
/* handle last line without a terminating newline (yuck) */
323-
if (linelen)
324-
nlines++;
325-
if (linelen > maxlength)
326-
maxlength = linelen;
327-
328-
/* set up the result and the line buffer */
348+
/* set up the result buffer */
329349
result = (char **) pg_malloc((nlines + 1) * sizeof(char *));
330-
buffer = (char *) pg_malloc(maxlength + 1);
331350

332-
/* now reprocess the file and store the lines */
333-
rewind(infile);
334-
nlines = 0;
335-
while (fgets(buffer, maxlength + 1, infile) != NULL)
336-
result[nlines++] = xstrdup(buffer);
351+
/* now split the buffer into lines */
352+
linebegin = buffer;
353+
n = 0;
354+
for (i = 0; i < len; i++)
355+
{
356+
if (buffer[i] == '\n' || i == len - 1)
357+
{
358+
int slen = &buffer[i] - linebegin + 1;
359+
char *linebuf = pg_malloc(slen + 1);
360+
memcpy(linebuf, linebegin, slen);
361+
linebuf[slen] = '\0';
362+
result[n++] = linebuf;
363+
linebegin = &buffer[i + 1];
364+
}
365+
}
366+
result[n] = NULL;
337367

338-
fclose(infile);
339368
free(buffer);
340-
result[nlines] = NULL;
341369

342370
return result;
343371
}

0 commit comments

Comments
 (0)
0