Skip to content

Commit b954b75

Browse files
committed
Tidy sanitizers' suppressions list and fix some mem leaks
Tihs PR makes a suppression list as specific as possible, so it won't cover up new issues. And adds comments to existing suppressions. Also, fixes memory leaks in pg_tde code (all related to frontend usage).
1 parent 34e2d29 commit b954b75

File tree

8 files changed

+119
-12
lines changed

8 files changed

+119
-12
lines changed

ci_scripts/suppressions/lsan.supp

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,52 @@
1+
# external submodule
2+
leak:kmip.c
3+
4+
# the parser of command line arguments of the backend
15
leak:save_ps_display_args
6+
7+
# a bunch of not freed addrinfo sctructs, escaped values etc.
28
leak:initdb.c
3-
leak:fe_memutils.c
4-
leak:fe-exec.c
5-
leak:fe-connect.c
6-
leak:pqexpbuffer.c
7-
leak:strdup
8-
leak:preproc.y
9-
leak:pgtypeslib/common.c
10-
leak:ecpglib/memory.c
11-
leak:kmip.c
9+
10+
# shlibs loaded with dlsym() never gets dlclose'd
11+
# TODO: needs an investigation as it is down the call stack
12+
# of exec_simple_query->PortalRun
13+
leak:internal_load_library
14+
15+
# pg_config utility does not free configdata
16+
leak:pg_config/pg_config.c
17+
18+
# A psprintf() result assigned to the global var pgdata_opt
19+
leak:bin/pg_ctl/pg_ctl.c
20+
21+
# TODO: A couple of not freed char *. Whilst wit's trivially to fix
22+
# meson complains with "maybe-uninitialized" on free()
23+
leak:bin/psql/describe.c
24+
25+
# options parsing during the regressions start
26+
leak:regression_main
27+
28+
# Static funcs in /bin/psql/startup.c are used
29+
# during the start to parse options
30+
leak:simple_action_list_append
31+
leak:parse_psql_options
32+
33+
# A bunch of parameters, options and identifiers are not being freed.
34+
#
35+
# TODO?: ReceiveArchiveStreamChunk->CreateBackupStreamer down in the
36+
# callstak creates a streamer that is never freed. It's being called in a loop
37+
# but it looks like it is not a repetitive action and rather happens once per
38+
# receiving CopyData message from server.
39+
#
40+
# bin/pg_basebackup/pg_basebackup.c
41+
leak:BaseBackup
42+
43+
# `varname` doesn't get freed in the case of warning and `continue` in the loop
44+
#
45+
# bin/psql/common.c
46+
leak:StoreQueryTuple
47+
48+
# does not free a bunch of parsed flags
49+
leak:bin/pg_waldump/pg_waldump.c
1250

1351
#pg_dump
1452
leak:getSchemaData

contrib/pg_tde/src/access/pg_tde_tdemap.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,6 +1077,10 @@ pg_tde_fetch_wal_keys(XLogRecPtr start_lsn)
10771077

10781078
wal_rec = pg_tde_add_wal_key_to_cache(&stub_key, InvalidXLogRecPtr);
10791079

1080+
#ifdef FRONTEND
1081+
/* The backen frees it after copying to the cache. */
1082+
pfree(principal_key);
1083+
#endif
10801084
LWLockRelease(lock_pk);
10811085
CloseTransientFile(fd);
10821086
return wal_rec;
@@ -1107,6 +1111,9 @@ pg_tde_fetch_wal_keys(XLogRecPtr start_lsn)
11071111
return_wal_rec = wal_rec;
11081112
}
11091113
}
1114+
#ifdef FRONTEND
1115+
pfree(principal_key);
1116+
#endif
11101117
LWLockRelease(lock_pk);
11111118
CloseTransientFile(fd);
11121119

contrib/pg_tde/src/catalog/tde_keyring.c

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,51 @@ check_provider_record(KeyringProviderRecord *provider_record)
562562

563563
#endif /* !FRONTEND */
564564

565+
void
566+
free_keyring(GenericKeyring *keyring)
567+
{
568+
FileKeyring *file = (FileKeyring *) keyring;
569+
VaultV2Keyring *vault = (VaultV2Keyring *) keyring;
570+
KmipKeyring *kmip = (KmipKeyring *) keyring;
571+
572+
switch (keyring->type)
573+
{
574+
case FILE_KEY_PROVIDER:
575+
if (file->file_name)
576+
pfree(file->file_name);
577+
break;
578+
case VAULT_V2_KEY_PROVIDER:
579+
if (vault->vault_ca_path)
580+
pfree(vault->vault_ca_path);
581+
if (vault->vault_mount_path)
582+
pfree(vault->vault_mount_path);
583+
if (vault->vault_token)
584+
pfree(vault->vault_token);
585+
if (vault->vault_token_path)
586+
pfree(vault->vault_token_path);
587+
if (vault->vault_url)
588+
pfree(vault->vault_url);
589+
break;
590+
case KMIP_KEY_PROVIDER:
591+
if (kmip->kmip_ca_path)
592+
pfree(kmip->kmip_ca_path);
593+
if (kmip->kmip_cert_path)
594+
pfree(kmip->kmip_cert_path);
595+
if (kmip->kmip_host)
596+
pfree(kmip->kmip_host);
597+
if (kmip->kmip_key_path)
598+
pfree(kmip->kmip_key_path);
599+
if (kmip->kmip_port)
600+
pfree(kmip->kmip_port);
601+
break;
602+
default:
603+
Assert(false);
604+
break;
605+
}
606+
607+
pfree(keyring);
608+
}
609+
565610
void
566611
write_key_provider_info(KeyringProviderRecordInFile *record, bool write_xlog)
567612
{
@@ -682,6 +727,8 @@ simple_list_free(SimplePtrList *list)
682727
pfree(cell);
683728
cell = next;
684729
}
730+
731+
pfree(list);
685732
}
686733
#endif /* FRONTEND */
687734

@@ -818,6 +865,8 @@ load_file_keyring_provider_options(char *keyring_options)
818865
ereport(WARNING,
819866
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
820867
errmsg("file path is missing in the keyring options"));
868+
869+
free_keyring((GenericKeyring *) file_keyring);
821870
return NULL;
822871
}
823872

@@ -845,6 +894,8 @@ load_vaultV2_keyring_provider_options(char *keyring_options)
845894
(vaultV2_keyring->vault_token_path != NULL && vaultV2_keyring->vault_token_path[0] != '\0') ? "" : " tokenPath",
846895
(vaultV2_keyring->vault_url != NULL && vaultV2_keyring->vault_url[0] != '\0') ? "" : " url",
847896
(vaultV2_keyring->vault_mount_path != NULL && vaultV2_keyring->vault_mount_path[0] != '\0') ? "" : " mountPath"));
897+
898+
free_keyring((GenericKeyring *) vaultV2_keyring);
848899
return NULL;
849900
}
850901

@@ -878,6 +929,8 @@ load_kmip_keyring_provider_options(char *keyring_options)
878929
(kmip_keyring->kmip_ca_path != NULL && kmip_keyring->kmip_ca_path[0] != '\0') ? "" : " caPath",
879930
(kmip_keyring->kmip_cert_path != NULL && kmip_keyring->kmip_cert_path[0] != '\0') ? "" : " certPath",
880931
(kmip_keyring->kmip_key_path != NULL && kmip_keyring->kmip_key_path[0] != '\0') ? "" : " keyPath"));
932+
933+
free_keyring((GenericKeyring *) kmip_keyring);
881934
return NULL;
882935
}
883936

@@ -946,7 +999,10 @@ debug_print_kerying(GenericKeyring *keyring)
946999
static inline void
9471000
get_keyring_infofile_path(char *resPath, Oid dbOid)
9481001
{
949-
join_path_components(resPath, pg_tde_get_data_dir(), psprintf(PG_TDE_KEYRING_FILENAME, dbOid));
1002+
char *fname = psprintf(PG_TDE_KEYRING_FILENAME, dbOid);
1003+
1004+
join_path_components(resPath, pg_tde_get_data_dir(), fname);
1005+
pfree(fname);
9501006
}
9511007

9521008
static int

contrib/pg_tde/src/catalog/tde_principal_key.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,7 @@ get_principal_key_from_keyring(Oid dbOid)
863863
Assert(dbOid == principalKey->keyInfo.databaseId);
864864

865865
pfree(keyInfo);
866-
pfree(keyring);
866+
free_keyring(keyring);
867867
pfree(principalKeyInfo);
868868

869869
return principalKey;

contrib/pg_tde/src/include/access/pg_tde_tdemap.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@ extern void pg_tde_create_wal_key(InternalKey *rel_key_data, const RelFileLocato
8181
static inline void
8282
pg_tde_set_db_file_path(Oid dbOid, char *path)
8383
{
84-
join_path_components(path, pg_tde_get_data_dir(), psprintf(PG_TDE_MAP_FILENAME, dbOid));
84+
char *fname = psprintf(PG_TDE_MAP_FILENAME, dbOid);
85+
86+
join_path_components(path, pg_tde_get_data_dir(), fname);
87+
pfree(fname);
8588
}
8689

8790
extern void pg_tde_save_smgr_key(RelFileLocator rel, const InternalKey *key);

contrib/pg_tde/src/include/catalog/tde_keyring.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,6 @@ extern void redo_key_provider_info(KeyringProviderRecordInFile *xlrec);
4040
extern void ParseKeyringJSONOptions(ProviderType provider_type,
4141
GenericKeyring *out_opts,
4242
char *in_buf, int buf_len);
43+
extern void free_keyring(GenericKeyring *keyring);
4344

4445
#endif /* TDE_KEYRING_H */

contrib/pg_tde/src/keyring/keyring_vault.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,5 +438,6 @@ json_resp_object_field_start(void *state, char *fname, bool isnull)
438438
break;
439439
}
440440

441+
pfree(fname);
441442
return JSON_SUCCESS;
442443
}

contrib/pg_tde/src/pg_tde_change_key_provider.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,7 @@ main(int argc, char *argv[])
249249
controlfile->state != DB_SHUTDOWNED_IN_RECOVERY)
250250
pg_fatal("cluster must be shut down");
251251

252+
pfree(controlfile);
252253
cptr = strcat(cptr, datadir);
253254
cptr = strcat(cptr, "/");
254255
cptr = strcat(cptr, PG_TDE_DATA_DIR);

0 commit comments

Comments
 (0)