Skip to content

Commit b76ddff

Browse files
dr0pdbdr0pdb
committed
[BACKPORT 2024.2][yugabyte#26547] YSQL: Fix use after free in ysql_dump by using pg_strdup for tablegroup_name
Summary: #### Backport Summary Fixed trivial merge conflicts due to the usage of `YbTableProperties` instead of `YbcTableProperties` on the master branch. #### Original Summary As part of D36859 / 0dbe7d6, backup and restore support for colocated tables when multiple tablespaces exist was introduced. Upon fetching the tablegroup_name from `pg_yb_tablegroup`, the value was read and assigned via `PQgetvalue` without copying. This led to a use-after-free bug when the tablegroup_name was later read in dumpTableSchema since the result from the SQL query is immediately cleared in the next line (`PQclear`). ``` [P-yb-controller-1] ==3037==ERROR: AddressSanitizer: heap-use-after-free on address 0x51d0002013e6 at pc 0x55615b0a1f92 bp 0x7fff92475970 sp 0x7fff92475118 [P-yb-controller-1] READ of size 8 at 0x51d0002013e6 thread T0 [P-yb-controller-1] #0 0x55615b0a1f91 in strcmp ${YB_LLVM_TOOLCHAIN_DIR}/src/llvm-project/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:470:5 [P-yb-controller-1] #1 0x55615b1b90ba in dumpTableSchema ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15789:8 [P-yb-controller-1] #2 0x55615b178163 in dumpTable ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15299:4 [P-yb-controller-1] #3 0x55615b178163 in dumpDumpableObject ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:10216:4 [P-yb-controller-1] #4 0x55615b178163 in main ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:1019:3 [P-yb-controller-1] #5 0x7f3c0184e7e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: fd70eb98f80391a177070fcb8d757a63fe49b802) [P-yb-controller-1] #6 0x55615b0894bd in _start (${BUILD_ROOT}/postgres/bin/ysql_dump+0x10d4bd) [P-yb-controller-1] [P-yb-controller-1] 0x51d0002013e6 is located 358 bytes inside of 2048-byte region [0x51d000201280,0x51d000201a80) [P-yb-controller-1] freed by thread T0 here: [P-yb-controller-1] #0 0x55615b127196 in free ${YB_LLVM_TOOLCHAIN_DIR}/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3 [P-yb-controller-1] #1 0x7f3c02d65e85 in PQclear ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:755:3 [P-yb-controller-1] #2 0x55615b1c0103 in getYbTablePropertiesAndReloptions ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:19108:4 [P-yb-controller-1] #3 0x55615b1b8fab in dumpTableSchema ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15765:3 [P-yb-controller-1] #4 0x55615b178163 in dumpTable ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15299:4 [P-yb-controller-1] #5 0x55615b178163 in dumpDumpableObject ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:10216:4 [P-yb-controller-1] #6 0x55615b178163 in main ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:1019:3 [P-yb-controller-1] #7 0x7f3c0184e7e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: fd70eb98f80391a177070fcb8d757a63fe49b802) [P-yb-controller-1] [P-yb-controller-1] previously allocated by thread T0 here: [P-yb-controller-1] #0 0x55615b12742f in malloc ${YB_LLVM_TOOLCHAIN_DIR}/src/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:68:3 [P-yb-controller-1] #1 0x7f3c02d680a7 in pqResultAlloc ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:633:28 [P-yb-controller-1] #2 0x7f3c02d81294 in getRowDescriptions ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-protocol3.c:544:4 [P-yb-controller-1] #3 0x7f3c02d7f793 in pqParseInput3 ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-protocol3.c:324:11 [P-yb-controller-1] #4 0x7f3c02d6bcc8 in parseInput ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:2014:2 [P-yb-controller-1] #5 0x7f3c02d6bcc8 in PQgetResult ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:2100:3 [P-yb-controller-1] #6 0x7f3c02d6cd87 in PQexecFinish ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:2417:19 [P-yb-controller-1] #7 0x7f3c02d6cd87 in PQexec ${YB_SRC_ROOT}/src/postgres/src/interfaces/libpq/fe-exec.c:2256:9 [P-yb-controller-1] #8 0x55615b1f45df in ExecuteSqlQuery ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_backup_db.c:296:8 [P-yb-controller-1] #9 0x55615b1f4213 in ExecuteSqlQueryForSingleRow ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_backup_db.c:311:8 [P-yb-controller-1] #10 0x55615b1c008d in getYbTablePropertiesAndReloptions ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:19102:10 [P-yb-controller-1] #11 0x55615b1b8fab in dumpTableSchema ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15765:3 [P-yb-controller-1] #12 0x55615b178163 in dumpTable ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:15299:4 [P-yb-controller-1] #13 0x55615b178163 in dumpDumpableObject ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:10216:4 [P-yb-controller-1] #14 0x55615b178163 in main ${YB_SRC_ROOT}/src/postgres/src/bin/pg_dump/pg_dump.c:1019:3 [P-yb-controller-1] #15 0x7f3c0184e7e4 in __libc_start_main (/lib64/libc.so.6+0x3a7e4) (BuildId: fd70eb98f80391a177070fcb8d757a63fe49b802) ``` This revision fixes the issue by using pg_strdup to make a copy of the string. Jira: DB-15915 Original commit: 7eea1de / D43386 Test Plan: ./yb_build.sh asan --cxx-test integration-tests_xcluster_ddl_replication-test --gtest_filter XClusterDDLReplicationTest.DDLReplicationTablesNotColocated Reviewers: aagrawal, skumar, mlillibridge, sergei Reviewed By: aagrawal Subscribers: yql, sergei Differential Revision: https://phorge.dev.yugabyte.com/D43418
1 parent 609cdcf commit b76ddff

1 file changed

Lines changed: 18 additions & 3 deletions

File tree

‎src/postgres/src/bin/pg_dump/pg_dump.c‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15915,6 +15915,17 @@ dumpTablegroup(Archive *fout, TablegroupInfo *tginfo)
1591515915
free(namecopy);
1591615916
}
1591715917

15918+
static void
15919+
freeYbTablePropertiesIfRequired(YbTableProperties yb_properties)
15920+
{
15921+
if (!yb_properties)
15922+
return;
15923+
15924+
if (yb_properties->tablegroup_name)
15925+
free(yb_properties->tablegroup_name);
15926+
free(yb_properties);
15927+
}
15928+
1591815929
/*
1591915930
* dumpTableSchema
1592015931
* write the declaration (not data) of one user-defined table or view
@@ -16057,7 +16068,7 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1605716068
(tbinfo->relkind == RELKIND_RELATION || tbinfo->relkind == RELKIND_INDEX
1605816069
|| tbinfo->relkind == RELKIND_MATVIEW || tbinfo->relkind == RELKIND_PARTITIONED_TABLE))
1605916070
{
16060-
yb_properties = (YbTableProperties) pg_malloc(sizeof(YbTablePropertiesData));
16071+
yb_properties = (YbTableProperties) pg_malloc0(sizeof(YbTablePropertiesData));
1606116072
}
1606216073
PQExpBuffer yb_reloptions = createPQExpBuffer();
1606316074
getYbTablePropertiesAndReloptions(fout, yb_properties, yb_reloptions,
@@ -16722,6 +16733,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1672216733
tbinfo->attfdwoptions[j]);
1672316734
}
1672416735
}
16736+
16737+
freeYbTablePropertiesIfRequired(yb_properties);
1672516738
}
1672616739

1672716740
/*
@@ -16940,7 +16953,7 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
1694016953
if (is_colocated_database && !is_legacy_colocated_database)
1694116954
{
1694216955
YbTableProperties yb_properties;
16943-
yb_properties = (YbTableProperties) pg_malloc(sizeof(YbTablePropertiesData));
16956+
yb_properties = (YbTableProperties) pg_malloc0(sizeof(YbTablePropertiesData));
1694416957
PQExpBuffer yb_reloptions = createPQExpBuffer();
1694516958
getYbTablePropertiesAndReloptions(fout, yb_properties, yb_reloptions,
1694616959
indxinfo->dobj.catId.oid, indxinfo->dobj.name, tbinfo->relkind);
@@ -16960,6 +16973,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
1696016973
}
1696116974
}
1696216975
destroyPQExpBuffer(yb_reloptions);
16976+
16977+
freeYbTablePropertiesIfRequired(yb_properties);
1696316978
}
1696416979
/* Plain secondary index */
1696516980
appendPQExpBuffer(q, "%s", indxinfo->indexdef);
@@ -19174,7 +19189,7 @@ getYbTablePropertiesAndReloptions(Archive *fout, YbTableProperties properties,
1917419189
res = ExecuteSqlQueryForSingleRow(fout, query->data);
1917519190
int i_grpname = PQfnumber(res, "grpname");
1917619191
properties->tablegroup_name =
19177-
PQgetisnull(res, 0, i_grpname) ? "" : PQgetvalue(res, 0, i_grpname);
19192+
PQgetisnull(res, 0, i_grpname) ? pg_strdup("") : pg_strdup(PQgetvalue(res, 0, i_grpname));
1917819193

1917919194
PQclear(res);
1918019195
destroyPQExpBuffer(query);

0 commit comments

Comments
 (0)