From bd65cb3cd48a7a5ce48b26f8031ad3968efed87e Mon Sep 17 00:00:00 2001 From: Masahiko Sawada Date: Tue, 11 Mar 2025 09:56:40 -0700 Subject: pg_logicalinspect: Fix possible crash when passing a directory path. Previously, pg_logicalinspect functions were too trusting of their input and blindly passed it to SnapBuildRestoreSnapshot(). If the input pointed to a directory, the server could a PANIC error while attempting to fsync_fname() with isdir=false on a directory. This commit adds validation checks for input filenames and passes the LSN extracted from the filename to SnapBuildRestoreSnapshot() instead of the filename itself. It also adds regression tests for various input patterns and permission checks. Bug: #18828 Reported-by: Robins Tharakan Co-authored-by: Bertrand Drouvot Co-authored-by: Masahiko Sawada Discussion: https://postgr.es/m/18828-0f4701c635064211@postgresql.org --- src/backend/replication/logical/snapbuild.c | 14 +++++++------- src/include/replication/snapbuild_internal.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/backend/replication/logical/snapbuild.c b/src/backend/replication/logical/snapbuild.c index bd0680dcbe5..b64e53de017 100644 --- a/src/backend/replication/logical/snapbuild.c +++ b/src/backend/replication/logical/snapbuild.c @@ -1691,12 +1691,17 @@ out: * If 'missing_ok' is true, will not throw an error if the file is not found. */ bool -SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, const char *path, +SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn, MemoryContext context, bool missing_ok) { int fd; pg_crc32c checksum; Size sz; + char path[MAXPGPATH]; + + sprintf(path, "%s/%X-%X.snap", + PG_LOGICAL_SNAPSHOTS_DIR, + LSN_FORMAT_ARGS(lsn)); fd = OpenTransientFile(path, O_RDONLY | PG_BINARY); @@ -1788,18 +1793,13 @@ static bool SnapBuildRestore(SnapBuild *builder, XLogRecPtr lsn) { SnapBuildOnDisk ondisk; - char path[MAXPGPATH]; /* no point in loading a snapshot if we're already there */ if (builder->state == SNAPBUILD_CONSISTENT) return false; - sprintf(path, "%s/%X-%X.snap", - PG_LOGICAL_SNAPSHOTS_DIR, - LSN_FORMAT_ARGS(lsn)); - /* validate and restore the snapshot to 'ondisk' */ - if (!SnapBuildRestoreSnapshot(&ondisk, path, builder->context, true)) + if (!SnapBuildRestoreSnapshot(&ondisk, lsn, builder->context, true)) return false; /* diff --git a/src/include/replication/snapbuild_internal.h b/src/include/replication/snapbuild_internal.h index 081b01b890a..3b915dc8793 100644 --- a/src/include/replication/snapbuild_internal.h +++ b/src/include/replication/snapbuild_internal.h @@ -193,7 +193,7 @@ typedef struct SnapBuildOnDisk /* variable amount of TransactionIds follows */ } SnapBuildOnDisk; -extern bool SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, const char *path, +extern bool SnapBuildRestoreSnapshot(SnapBuildOnDisk *ondisk, XLogRecPtr lsn, MemoryContext context, bool missing_ok); #endif /* SNAPBUILD_INTERNAL_H */ -- cgit v1.2.3