Skip to content

Commit ccb0e6b

Browse files
authored
Fix a memory leak in uri_contains_password. (#687)
* Fix a memory leak in uri_contains_password. * Add a fix to tmux_prepare_XDG_environment memory leak. * Fix another connection string parsing memory leak. * Fix calling PQsetNoticeProcessor on a finished connection. * Fix a leak in parse_version_number. * Add missed free_program calls before exit in tmux calling code. * Fix some more leaks reported by valgrind. * Fix pg_setup_get_username and make it "cache invalidation" style. * Fix another leak found by valgrind, in the tmux code path.
1 parent 5121a16 commit ccb0e6b

4 files changed

Lines changed: 38 additions & 14 deletions

File tree

src/bin/pg_autoctl/cli_do_tmux.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,13 +554,15 @@ tmux_prepare_XDG_environment(const char *root, bool createDirectories)
554554
if (pg_mkdir_p(env, 0700) == -1)
555555
{
556556
log_error("mkdir -p \"%s\": %m", env);
557+
free(env);
557558
return false;
558559
}
559560
}
560561

561562
if (!normalize_filename(env, env, MAXPGPATH))
562563
{
563564
/* errors have already been logged */
565+
free(env);
564566
return false;
565567
}
566568

@@ -588,9 +590,13 @@ tmux_prepare_XDG_environment(const char *root, bool createDirectories)
588590
if (pg_mkdir_p(targetPath, 0700) == -1)
589591
{
590592
log_error("mkdir -p \"%s\": %m", targetPath);
593+
594+
free(env);
591595
return false;
592596
}
593597
}
598+
599+
free(env);
594600
}
595601

596602
return true;
@@ -894,6 +900,7 @@ tmux_start_server(const char *scriptName)
894900
(void) execute_subprogram(&program);
895901

896902
/* we only get there when the tmux session is done */
903+
free_program(&program);
897904
return true;
898905
}
899906

@@ -935,6 +942,8 @@ tmux_attach_session(const char *tmux_path, const char *sessionName)
935942
(void) execute_subprogram(&program);
936943

937944
/* we only get there when the tmux session is done */
945+
free_program(&program);
946+
938947
return true;
939948
}
940949

@@ -1477,6 +1486,7 @@ cli_do_tmux_wait(int argc, char **argv)
14771486
(void) snprintf_program_command_line(&program, command, BUFSIZE);
14781487

14791488
log_error("%s [%d]", command, program.returnCode);
1489+
free_program(&program);
14801490
exit(EXIT_CODE_INTERNAL_ERROR);
14811491
}
14821492

@@ -1501,8 +1511,12 @@ cli_do_tmux_wait(int argc, char **argv)
15011511
{
15021512
log_info("The monitor is ready at: %s", showUri.stdOut);
15031513
}
1514+
1515+
free_program(&showUri);
15041516
}
15051517

1518+
free_program(&program);
1519+
15061520
if (!ready)
15071521
{
15081522
exit(EXIT_CODE_INTERNAL_ERROR);

src/bin/pg_autoctl/parsing.c

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ parse_version_number(const char *version_string,
140140
return false;
141141
}
142142

143+
free(match);
143144
return true;
144145
}
145146

@@ -701,6 +702,8 @@ parse_pguri_info_key_vals(const char *pguri,
701702
}
702703
}
703704

705+
PQconninfoFree(conninfo);
706+
704707
/*
705708
* Display an error message per missing field, and only then return false
706709
* if we're missing any one of those.
@@ -1044,8 +1047,9 @@ parseNodesArray(const char *nodesJSON,
10441047

10451048

10461049
/*
1047-
* uri_contains_password takes a Postgres connection string and checks to see if it contains a parameter
1048-
* called password. Returns true if a password keyword is present in the connection string.
1050+
* uri_contains_password takes a Postgres connection string and checks to see
1051+
* if it contains a parameter called password. Returns true if a password
1052+
* keyword is present in the connection string.
10491053
*/
10501054
static bool
10511055
uri_contains_password(const char *pguri)
@@ -1067,16 +1071,16 @@ uri_contains_password(const char *pguri)
10671071
*/
10681072
for (option = conninfo; option->keyword != NULL; option++)
10691073
{
1070-
if (
1071-
strcmp(option->keyword, "password") == 0 &&
1074+
if (strcmp(option->keyword, "password") == 0 &&
10721075
option->val != NULL &&
1073-
!IS_EMPTY_STRING_BUFFER(option->val)
1074-
)
1076+
!IS_EMPTY_STRING_BUFFER(option->val))
10751077
{
1078+
PQconninfoFree(conninfo);
10761079
return true;
10771080
}
10781081
}
10791082

1083+
PQconninfoFree(conninfo);
10801084
return false;
10811085
}
10821086

src/bin/pg_autoctl/pgsetup.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1342,7 +1342,7 @@ pg_setup_role(PostgresSetup *pgSetup)
13421342
char *
13431343
pg_setup_get_username(PostgresSetup *pgSetup)
13441344
{
1345-
char userEnv[NAMEDATALEN];
1345+
char userEnv[NAMEDATALEN] = { 0 };
13461346

13471347
/* use a configured username if provided */
13481348
if (!IS_EMPTY_STRING_BUFFER(pgSetup->username))
@@ -1359,21 +1359,24 @@ pg_setup_get_username(PostgresSetup *pgSetup)
13591359
{
13601360
log_trace("username found in passwd: %s", pw->pw_name);
13611361

1362-
/* struct passwd is in thread shared space, return a copy */
1363-
return strdup(pw->pw_name);
1362+
strlcpy(pgSetup->username, pw->pw_name, sizeof(pgSetup->username));
1363+
return pgSetup->username;
13641364
}
13651365

13661366

13671367
/* fallback on USER from env if the user cannot be found in passwd */
13681368
if (get_env_copy("USER", userEnv, NAMEDATALEN))
13691369
{
13701370
log_trace("username found in USER environment variable: %s", userEnv);
1371-
return strdup(userEnv);
1371+
1372+
strlcpy(pgSetup->username, userEnv, sizeof(pgSetup->username));
1373+
return pgSetup->username;
13721374
}
13731375

13741376
log_trace("username fallback to default: %s", DEFAULT_USERNAME);
1377+
strlcpy(pgSetup->username, DEFAULT_USERNAME, sizeof(pgSetup->username));
13751378

1376-
return DEFAULT_USERNAME;
1379+
return pgSetup->username;
13771380
}
13781381

13791382

src/bin/pg_autoctl/pgsql.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2257,13 +2257,16 @@ pgsql_create_user(PGSQL *pgsql, const char *userName, const char *password,
22572257

22582258
PQclear(result);
22592259
clear_results(pgsql);
2260+
22602261
if (pgsql->connectionStatementType == PGSQL_CONNECTION_SINGLE_STATEMENT)
22612262
{
22622263
pgsql_finish(pgsql);
22632264
}
2264-
2265-
/* restore the normal notice message processing, if needed. */
2266-
PQsetNoticeProcessor(connection, previousNoticeProcessor, NULL);
2265+
else
2266+
{
2267+
/* restore the normal notice message processing, if needed. */
2268+
PQsetNoticeProcessor(connection, previousNoticeProcessor, NULL);
2269+
}
22672270

22682271
return true;
22692272
}

0 commit comments

Comments
 (0)