From 052cc223d5ce1b727f62afff75797c88d82f880b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 30 Aug 2016 18:22:43 -0400 Subject: [PATCH] Fix a bunch of places that called malloc and friends with no NULL check. Where possible, use palloc or pg_malloc instead; otherwise, insert explicit NULL checks. Generally speaking, these are places where an actual OOM is quite unlikely, either because they're in client programs that don't allocate all that much, or they're very early in process startup so that we'd likely have had a fork() failure instead. Hence, no back-patch, even though this is nominally a bug fix. Michael Paquier, with some adjustments by me Discussion: --- contrib/pg_standby/pg_standby.c | 2 +- contrib/vacuumlo/vacuumlo.c | 8 ++-- src/backend/bootstrap/bootstrap.c | 14 ++---- src/backend/port/dynloader/darwin.c | 3 ++ src/backend/utils/misc/ps_status.c | 27 +++++++++++ src/bin/pg_archivecleanup/pg_archivecleanup.c | 2 +- src/bin/psql/command.c | 12 ++++- src/common/exec.c | 9 +++- src/test/isolation/isolationtester.c | 14 +++--- src/test/isolation/specparse.y | 34 ++++++------- src/test/isolation/specscanner.l | 4 +- src/test/regress/pg_regress.c | 48 +++++++++---------- 12 files changed, 108 insertions(+), 69 deletions(-) diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c index 5eac2b1e49..e4136f9149 100644 --- a/contrib/pg_standby/pg_standby.c +++ b/contrib/pg_standby/pg_standby.c @@ -632,7 +632,7 @@ main(int argc, char **argv) } break; case 't': /* Trigger file */ - triggerPath = strdup(optarg); + triggerPath = pg_strdup(optarg); break; case 'w': /* Max wait time */ maxwaittime = atoi(optarg); diff --git a/contrib/vacuumlo/vacuumlo.c b/contrib/vacuumlo/vacuumlo.c index 0a9328dc0e..221e1dbcb1 100644 --- a/contrib/vacuumlo/vacuumlo.c +++ b/contrib/vacuumlo/vacuumlo.c @@ -242,7 +242,7 @@ vacuumlo(const char *database, const struct _param * param) if (!schema || !table || !field) { - fprintf(stderr, "Out of memory\n"); + fprintf(stderr, "%s", PQerrorMessage(conn)); PQclear(res); PQfinish(conn); if (schema != NULL) @@ -519,7 +519,7 @@ main(int argc, char **argv) } break; case 'U': - param.pg_user = strdup(optarg); + param.pg_user = pg_strdup(optarg); break; case 'w': param.pg_prompt = TRI_NO; @@ -534,10 +534,10 @@ main(int argc, char **argv) fprintf(stderr, "%s: invalid port number: %s\n", progname, optarg); exit(1); } - param.pg_port = strdup(optarg); + param.pg_port = pg_strdup(optarg); break; case 'h': - param.pg_host = strdup(optarg); + param.pg_host = pg_strdup(optarg); break; } } diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 8feeae05df..3870a4deb9 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -47,7 +47,8 @@ uint32 bootstrap_data_checksum_version = 0; /* No checksum */ -#define ALLOC(t, c) ((t *) calloc((unsigned)(c), sizeof(t))) +#define ALLOC(t, c) \ + ((t *) MemoryContextAllocZero(TopMemoryContext, (unsigned)(c) * sizeof(t))) static void CheckerModeMain(void); static void BootstrapModeMain(void); @@ -227,7 +228,7 @@ AuxiliaryProcessMain(int argc, char *argv[]) SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV); break; case 'D': - userDoption = strdup(optarg); + userDoption = pstrdup(optarg); break; case 'd': { @@ -1002,13 +1003,8 @@ boot_get_type_io_data(Oid typid, static Form_pg_attribute AllocateAttribute(void) { - Form_pg_attribute attribute = (Form_pg_attribute) malloc(ATTRIBUTE_FIXED_PART_SIZE); - - if (!PointerIsValid(attribute)) - elog(FATAL, "out of memory"); - MemSet(attribute, 0, ATTRIBUTE_FIXED_PART_SIZE); - - return attribute; + return (Form_pg_attribute) + MemoryContextAllocZero(TopMemoryContext, ATTRIBUTE_FIXED_PART_SIZE); } /* diff --git a/src/backend/port/dynloader/darwin.c b/src/backend/port/dynloader/darwin.c index ccd92c39d4..a83c614f4f 100644 --- a/src/backend/port/dynloader/darwin.c +++ b/src/backend/port/dynloader/darwin.c @@ -78,6 +78,9 @@ pg_dlsym(void *handle, char *funcname) NSSymbol symbol; char *symname = (char *) malloc(strlen(funcname) + 2); + if (!symname) + return NULL; + sprintf(symname, "_%s", funcname); if (NSIsSymbolNameDefined(symname)) { diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c index 892a810bab..c50be8aab6 100644 --- a/src/backend/utils/misc/ps_status.c +++ b/src/backend/utils/misc/ps_status.c @@ -113,6 +113,9 @@ static char **save_argv; * overwritten during init_ps_display. Also, the physical location of the * environment strings may be moved, so this should be called before any code * that might try to hang onto a getenv() result.) + * + * Note that in case of failure this cannot call elog() as that is not + * initialized yet. We rely on write_stderr() instead. */ char ** save_ps_display_args(int argc, char **argv) @@ -163,8 +166,20 @@ save_ps_display_args(int argc, char **argv) * move the environment out of the way */ new_environ = (char **) malloc((i + 1) * sizeof(char *)); + if (!new_environ) + { + write_stderr("out of memory\n"); + exit(1); + } for (i = 0; environ[i] != NULL; i++) + { new_environ[i] = strdup(environ[i]); + if (!new_environ[i]) + { + write_stderr("out of memory\n"); + exit(1); + } + } new_environ[i] = NULL; environ = new_environ; } @@ -189,8 +204,20 @@ save_ps_display_args(int argc, char **argv) int i; new_argv = (char **) malloc((argc + 1) * sizeof(char *)); + if (!new_argv) + { + write_stderr("out of memory\n"); + exit(1); + } for (i = 0; i < argc; i++) + { new_argv[i] = strdup(argv[i]); + if (!new_argv[i]) + { + write_stderr("out of memory\n"); + exit(1); + } + } new_argv[argc] = NULL; #if defined(__darwin__) diff --git a/src/bin/pg_archivecleanup/pg_archivecleanup.c b/src/bin/pg_archivecleanup/pg_archivecleanup.c index 2b3d15dd58..319038fc80 100644 --- a/src/bin/pg_archivecleanup/pg_archivecleanup.c +++ b/src/bin/pg_archivecleanup/pg_archivecleanup.c @@ -314,7 +314,7 @@ main(int argc, char **argv) dryrun = true; break; case 'x': - additional_ext = strdup(optarg); /* Extension to remove + additional_ext = pg_strdup(optarg); /* Extension to remove * from xlogfile names */ break; default: diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 38038d415f..a9a2fdbf26 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1182,15 +1182,23 @@ exec_command(const char *cmd, fflush(stdout); } result = gets_fromFile(stdin); + if (!result) + { + psql_error("\\%s: could not read value for variable\n", + cmd); + success = false; + } } - if (!SetVariable(pset.vars, opt, result)) + if (result && + !SetVariable(pset.vars, opt, result)) { psql_error("\\%s: error while setting variable\n", cmd); success = false; } - free(result); + if (result) + free(result); if (prompt_text) free(prompt_text); free(opt); diff --git a/src/common/exec.c b/src/common/exec.c index d736b02280..a2de0718cf 100644 --- a/src/common/exec.c +++ b/src/common/exec.c @@ -553,6 +553,7 @@ set_pglocale_pgservice(const char *argv0, const char *app) char my_exec_path[MAXPGPATH]; char env_path[MAXPGPATH + sizeof("PGSYSCONFDIR=")]; /* longer than * PGLOCALEDIR */ + char *dup_path; /* don't set LC_ALL in the backend */ if (strcmp(app, PG_TEXTDOMAIN("postgres")) != 0) @@ -583,7 +584,9 @@ set_pglocale_pgservice(const char *argv0, const char *app) /* set for libpq to use */ snprintf(env_path, sizeof(env_path), "PGLOCALEDIR=%s", path); canonicalize_path(env_path + 12); - putenv(strdup(env_path)); + dup_path = strdup(env_path); + if (dup_path) + putenv(dup_path); } #endif @@ -594,7 +597,9 @@ set_pglocale_pgservice(const char *argv0, const char *app) /* set for libpq to use */ snprintf(env_path, sizeof(env_path), "PGSYSCONFDIR=%s", path); canonicalize_path(env_path + 13); - putenv(strdup(env_path)); + dup_path = strdup(env_path); + if (dup_path) + putenv(dup_path); } } diff --git a/src/test/isolation/isolationtester.c b/src/test/isolation/isolationtester.c index 908a7ce800..db2b55982b 100644 --- a/src/test/isolation/isolationtester.c +++ b/src/test/isolation/isolationtester.c @@ -119,7 +119,7 @@ main(int argc, char **argv) for (i = 0; i < testspec->nsessions; i++) nallsteps += testspec->sessions[i]->nsteps; - allsteps = malloc(nallsteps * sizeof(Step *)); + allsteps = pg_malloc(nallsteps * sizeof(Step *)); n = 0; for (i = 0; i < testspec->nsessions; i++) @@ -190,7 +190,7 @@ main(int argc, char **argv) if (PQresultStatus(res) == PGRES_TUPLES_OK) { if (PQntuples(res) == 1 && PQnfields(res) == 1) - backend_pids[i] = strdup(PQgetvalue(res, 0, 0)); + backend_pids[i] = pg_strdup(PQgetvalue(res, 0, 0)); else { fprintf(stderr, "backend pid query returned %d rows and %d columns, expected 1 row and 1 column", @@ -286,7 +286,7 @@ run_all_permutations(TestSpec *testspec) for (i = 0; i < testspec->nsessions; i++) nsteps += testspec->sessions[i]->nsteps; - steps = malloc(sizeof(Step *) * nsteps); + steps = pg_malloc(sizeof(Step *) * nsteps); /* * To generate the permutations, we conceptually put the steps of each @@ -297,7 +297,7 @@ run_all_permutations(TestSpec *testspec) * A pile is actually just an integer which tells how many steps we've * already picked from this pile. */ - piles = malloc(sizeof(int) * testspec->nsessions); + piles = pg_malloc(sizeof(int) * testspec->nsessions); for (i = 0; i < testspec->nsessions; i++) piles[i] = 0; @@ -345,7 +345,7 @@ run_named_permutations(TestSpec *testspec) Permutation *p = testspec->permutations[i]; Step **steps; - steps = malloc(p->nsteps * sizeof(Step *)); + steps = pg_malloc(p->nsteps * sizeof(Step *)); /* Find all the named steps using the lookup table */ for (j = 0; j < p->nsteps; j++) @@ -476,8 +476,8 @@ run_permutation(TestSpec *testspec, int nsteps, Step **steps) return; } - waiting = malloc(sizeof(Step *) * testspec->nsessions); - errorstep = malloc(sizeof(Step *) * testspec->nsessions); + waiting = pg_malloc(sizeof(Step *) * testspec->nsessions); + errorstep = pg_malloc(sizeof(Step *) * testspec->nsessions); printf("\nstarting permutation:"); for (i = 0; i < nsteps; i++) diff --git a/src/test/isolation/specparse.y b/src/test/isolation/specparse.y index fce6cc6c94..59744cea54 100644 --- a/src/test/isolation/specparse.y +++ b/src/test/isolation/specparse.y @@ -73,8 +73,8 @@ setup_list: } | setup_list setup { - $$.elements = realloc($1.elements, - ($1.nelements + 1) * sizeof(void *)); + $$.elements = pg_realloc($1.elements, + ($1.nelements + 1) * sizeof(void *)); $$.elements[$1.nelements] = $2; $$.nelements = $1.nelements + 1; } @@ -97,15 +97,15 @@ opt_teardown: session_list: session_list session { - $$.elements = realloc($1.elements, - ($1.nelements + 1) * sizeof(void *)); + $$.elements = pg_realloc($1.elements, + ($1.nelements + 1) * sizeof(void *)); $$.elements[$1.nelements] = $2; $$.nelements = $1.nelements + 1; } | session { $$.nelements = 1; - $$.elements = malloc(sizeof(void *)); + $$.elements = pg_malloc(sizeof(void *)); $$.elements[0] = $1; } ; @@ -113,7 +113,7 @@ session_list: session: SESSION string_literal opt_setup step_list opt_teardown { - $$ = malloc(sizeof(Session)); + $$ = pg_malloc(sizeof(Session)); $$->name = $2; $$->setupsql = $3; $$->steps = (Step **) $4.elements; @@ -125,15 +125,15 @@ session: step_list: step_list step { - $$.elements = realloc($1.elements, - ($1.nelements + 1) * sizeof(void *)); + $$.elements = pg_realloc($1.elements, + ($1.nelements + 1) * sizeof(void *)); $$.elements[$1.nelements] = $2; $$.nelements = $1.nelements + 1; } | step { $$.nelements = 1; - $$.elements = malloc(sizeof(void *)); + $$.elements = pg_malloc(sizeof(void *)); $$.elements[0] = $1; } ; @@ -142,7 +142,7 @@ step_list: step: STEP string_literal sqlblock { - $$ = malloc(sizeof(Step)); + $$ = pg_malloc(sizeof(Step)); $$->name = $2; $$->sql = $3; $$->errormsg = NULL; @@ -164,15 +164,15 @@ opt_permutation_list: permutation_list: permutation_list permutation { - $$.elements = realloc($1.elements, - ($1.nelements + 1) * sizeof(void *)); + $$.elements = pg_realloc($1.elements, + ($1.nelements + 1) * sizeof(void *)); $$.elements[$1.nelements] = $2; $$.nelements = $1.nelements + 1; } | permutation { $$.nelements = 1; - $$.elements = malloc(sizeof(void *)); + $$.elements = pg_malloc(sizeof(void *)); $$.elements[0] = $1; } ; @@ -181,7 +181,7 @@ permutation_list: permutation: PERMUTATION string_literal_list { - $$ = malloc(sizeof(Permutation)); + $$ = pg_malloc(sizeof(Permutation)); $$->stepnames = (char **) $2.elements; $$->nsteps = $2.nelements; } @@ -190,15 +190,15 @@ permutation: string_literal_list: string_literal_list string_literal { - $$.elements = realloc($1.elements, - ($1.nelements + 1) * sizeof(void *)); + $$.elements = pg_realloc($1.elements, + ($1.nelements + 1) * sizeof(void *)); $$.elements[$1.nelements] = $2; $$.nelements = $1.nelements + 1; } | string_literal { $$.nelements = 1; - $$.elements = malloc(sizeof(void *)); + $$.elements = pg_malloc(sizeof(void *)); $$.elements[0] = $1; } ; diff --git a/src/test/isolation/specscanner.l b/src/test/isolation/specscanner.l index 675bd21f17..6f081d567c 100644 --- a/src/test/isolation/specscanner.l +++ b/src/test/isolation/specscanner.l @@ -56,7 +56,7 @@ teardown { return(TEARDOWN); } } \" { litbuf[litbufpos] = '\0'; - yylval.str = strdup(litbuf); + yylval.str = pg_strdup(litbuf); BEGIN(INITIAL); return(string_literal); } @@ -72,7 +72,7 @@ teardown { return(TEARDOWN); } } {space}*"}" { litbuf[litbufpos] = '\0'; - yylval.str = strdup(litbuf); + yylval.str = pg_strdup(litbuf); BEGIN(INITIAL); return(sqlblock); } diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index 574f5b87be..2260057840 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -151,10 +151,10 @@ unlimit_core_size(void) void add_stringlist_item(_stringlist **listhead, const char *str) { - _stringlist *newentry = malloc(sizeof(_stringlist)); + _stringlist *newentry = pg_malloc(sizeof(_stringlist)); _stringlist *oldentry; - newentry->str = strdup(str); + newentry->str = pg_strdup(str); newentry->next = NULL; if (*listhead == NULL) *listhead = newentry; @@ -187,7 +187,7 @@ free_stringlist(_stringlist **listhead) static void split_to_stringlist(const char *s, const char *delim, _stringlist **listhead) { - char *sc = strdup(s); + char *sc = pg_strdup(s); char *token = strtok(sc, delim); while (token) @@ -327,7 +327,7 @@ signal_remove_temp(int signum) static const char * make_temp_sockdir(void) { - char *template = strdup("/tmp/pg_regress-XXXXXX"); + char *template = pg_strdup("/tmp/pg_regress-XXXXXX"); temp_sockdir = mkdtemp(template); if (temp_sockdir == NULL) @@ -441,7 +441,7 @@ replace_string(char *string, char *replace, char *replacement) while ((ptr = strstr(string, replace)) != NULL) { - char *dup = strdup(string); + char *dup = pg_strdup(string); strlcpy(string, dup, ptr - string + 1); strcat(string, replacement); @@ -661,11 +661,11 @@ load_resultmap(void) */ if (string_matches_pattern(host_platform, platform)) { - _resultmap *entry = malloc(sizeof(_resultmap)); + _resultmap *entry = pg_malloc(sizeof(_resultmap)); - entry->test = strdup(buf); - entry->type = strdup(file_type); - entry->resultfile = strdup(expected); + entry->test = pg_strdup(buf); + entry->type = pg_strdup(file_type); + entry->resultfile = pg_strdup(expected); entry->next = resultmap; resultmap = entry; } @@ -908,7 +908,7 @@ current_windows_user(const char **acct, const char **dom) progname, GetLastError()); exit(2); } - tokenuser = malloc(retlen); + tokenuser = pg_malloc(retlen); if (!GetTokenInformation(token, TokenUser, tokenuser, retlen, &retlen)) { fprintf(stderr, @@ -1460,7 +1460,7 @@ wait_for_tests(PID_TYPE * pids, int *statuses, char **names, int num_tests) int i; #ifdef WIN32 - PID_TYPE *active_pids = malloc(num_tests * sizeof(PID_TYPE)); + PID_TYPE *active_pids = pg_malloc(num_tests * sizeof(PID_TYPE)); memcpy(active_pids, pids, num_tests * sizeof(PID_TYPE)); #endif @@ -1848,7 +1848,7 @@ open_result_files(void) /* create the log file (copy of running status output) */ snprintf(file, sizeof(file), "%s/regression.out", outputdir); - logfilename = strdup(file); + logfilename = pg_strdup(file); logfile = fopen(logfilename, "w"); if (!logfile) { @@ -1859,7 +1859,7 @@ open_result_files(void) /* create the diffs file as empty */ snprintf(file, sizeof(file), "%s/regression.diffs", outputdir); - difffilename = strdup(file); + difffilename = pg_strdup(file); difffile = fopen(difffilename, "w"); if (!difffile) { @@ -2064,13 +2064,13 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc * before we add the specified one. */ free_stringlist(&dblist); - split_to_stringlist(strdup(optarg), ", ", &dblist); + split_to_stringlist(pg_strdup(optarg), ", ", &dblist); break; case 2: debug = true; break; case 3: - inputdir = strdup(optarg); + inputdir = pg_strdup(optarg); break; case 4: add_stringlist_item(&loadlanguage, optarg); @@ -2079,10 +2079,10 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc max_connections = atoi(optarg); break; case 6: - encoding = strdup(optarg); + encoding = pg_strdup(optarg); break; case 7: - outputdir = strdup(optarg); + outputdir = pg_strdup(optarg); break; case 8: add_stringlist_item(&schedulelist, optarg); @@ -2094,27 +2094,27 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc nolocale = true; break; case 13: - hostname = strdup(optarg); + hostname = pg_strdup(optarg); break; case 14: port = atoi(optarg); port_specified_by_user = true; break; case 15: - user = strdup(optarg); + user = pg_strdup(optarg); break; case 16: /* "--bindir=" means to use PATH */ if (strlen(optarg)) - bindir = strdup(optarg); + bindir = pg_strdup(optarg); else bindir = NULL; break; case 17: - dlpath = strdup(optarg); + dlpath = pg_strdup(optarg); break; case 18: - split_to_stringlist(strdup(optarg), ", ", &extraroles); + split_to_stringlist(pg_strdup(optarg), ", ", &extraroles); break; case 19: add_stringlist_item(&temp_configs, optarg); @@ -2123,13 +2123,13 @@ regression_main(int argc, char *argv[], init_function ifunc, test_function tfunc use_existing = true; break; case 21: - launcher = strdup(optarg); + launcher = pg_strdup(optarg); break; case 22: add_stringlist_item(&loadextension, optarg); break; case 24: - config_auth_datadir = pstrdup(optarg); + config_auth_datadir = pg_strdup(optarg); break; default: /* getopt_long already emitted a complaint */