Handle close() failures more robustly in pg_dump and pg_basebackup.

Coverity complained that applying get_gz_error after a failed gzclose,
as we did in one place in pg_basebackup, is unsafe.  I think it's
right: it's entirely likely that the call is touching freed memory.
Change that to inspect errno, as we do for other gzclose calls.

Also, be careful to initialize errno to zero immediately before any
gzclose() call where we care about the error status.  (There are
some calls where we don't, because we already failed at some previous
step.)  This ensures that we don't get a misleadingly irrelevant
error code if gzclose() fails in a way that doesn't set errno.
We could work harder at that, but it looks to me like all such cases
are basically can't-happen if we're not misusing zlib, so it's
not worth the extra notational cruft that would be required.

Also, fix several places that simply failed to check for close-time
errors at all, mostly at some remove from the close or gzclose itself;
and one place that did check but didn't bother to report the errno.

Back-patch to v12.  These mistakes are older than that, but between
the frontend logging API changes that happened in v12 and the fact
that frontend code can't rely on %m before that, the patch would need
substantial revision to work in older branches.  It doesn't quite
seem worth the trouble given the lack of related field complaints.

Patch by me; thanks to Michael Paquier for review.

Discussion: https://postgr.es/m/1343113.1636489231@sss.pgh.pa.us

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index ac07a0ef009366ebcac9234888dc7af2a86f199d..095953b1c9644daa9a8af750c645ce3295117577 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1160,10 +1160,11 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 #ifdef HAVE_LIBZ
                        if (ztarfile != NULL)
                        {
+                               errno = 0;              /* in case gzclose() doesn't set it */
                                if (gzclose(ztarfile) != 0)
                                {
-                                       pg_log_error("could not close compressed file \"%s\": %s",
-                                                                filename, get_gz_error(ztarfile));
+                                       pg_log_error("could not close compressed file \"%s\": %m",
+                                                                filename);
                                        exit(1);
                                }
                        }

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index de0d9ae0d0553adc694b95a2037f22246ed03bd8..8dfce7d1ca56d069d9776b0fe401ae95f7bd6938 100644 (file)
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -74,7 +74,12 @@ mark_file_as_archived(StreamCtl *stream, const char *fname)
                return false;
        }
 
-       stream->walmethod->close(f, CLOSE_NORMAL);
+       if (stream->walmethod->close(f, CLOSE_NORMAL) != 0)
+       {
+               pg_log_error("could not close archive status file \"%s\": %s",
+                                        tmppath, stream->walmethod->getlasterror());
+               return false;
+       }
 
        return true;
 }

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index f55651e4e51099d9a657e7adc37aea2600fcc826..c437b68aca0601492cbd6d47f0ecb903a91ae359 100644 (file)
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -236,7 +236,10 @@ dir_close(Walfile f, WalCloseMethod method)
 
 #ifdef HAVE_LIBZ
        if (dir_data->compression > 0)
+       {
+               errno = 0;                              /* in case gzclose() doesn't set it */
                r = gzclose(df->gzfp);
+       }
        else
 #endif
                r = close(df->fd);

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index d98623e7f07bdccc8bd9c035d154f5b878ba43b5..651adfdcc953cf3c777e78c505d010ae56372a53 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -269,6 +269,7 @@ CloseArchive(Archive *AHX)
        AH->ClosePtr(AH);
 
        /* Close the output */
+       errno = 0;                                      /* in case gzclose() doesn't set it */
        if (AH->gzOut)
                res = GZCLOSE(AH->OF);
        else if (AH->OF != stdout)
@@ -1582,6 +1583,7 @@ RestoreOutput(ArchiveHandle *AH, OutputContext savedContext)
 {
        int                     res;
 
+       errno = 0;                                      /* in case gzclose() doesn't set it */
        if (AH->gzOut)
                res = GZCLOSE(AH->OF);
        else

diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c
index 90670a6e209076fda22b95cd0b42c052cea17842..eef864df2116053334c77c6871ae8502a659270e 100644 (file)
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -371,7 +371,8 @@ _EndData(ArchiveHandle *AH, TocEntry *te)
        lclContext *ctx = (lclContext *) AH->formatData;
 
        /* Close the file */
-       cfclose(ctx->dataFH);
+       if (cfclose(ctx->dataFH) != 0)
+               fatal("could not close data file: %m");
 
        ctx->dataFH = NULL;
 }
@@ -686,7 +687,8 @@ _EndBlob(ArchiveHandle *AH, TocEntry *te, Oid oid)
        int                     len;
 
        /* Close the BLOB data file itself */
-       cfclose(ctx->dataFH);
+       if (cfclose(ctx->dataFH) != 0)
+               fatal("could not close blob data file: %m");
        ctx->dataFH = NULL;
 
        /* register the blob in blobs.toc */
@@ -705,7 +707,8 @@ _EndBlobs(ArchiveHandle *AH, TocEntry *te)
 {
        lclContext *ctx = (lclContext *) AH->formatData;
 
-       cfclose(ctx->blobsTocFH);
+       if (cfclose(ctx->blobsTocFH) != 0)
+               fatal("could not close blobs TOC file: %m");
        ctx->blobsTocFH = NULL;
 }
 

diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c
index 7e7625a39dec999f6e8f256d4a3f19dc5249abd0..8c4037aca1ad3dbfa363b490527749698f51ae70 100644 (file)
--- a/src/bin/pg_dump/pg_backup_tar.c
+++ b/src/bin/pg_dump/pg_backup_tar.c
@@ -438,8 +438,11 @@ tarClose(ArchiveHandle *AH, TAR_MEMBER *th)
         * Close the GZ file since we dup'd. This will flush the buffers.
         */
        if (AH->compression != 0)
+       {
+               errno = 0;                              /* in case gzclose() doesn't set it */
                if (GZCLOSE(th->zFH) != 0)
-                       fatal("could not close tar member");
+                       fatal("could not close tar member: %m");
+       }
 
        if (th->mode == 'w')
                _tarAddFile(AH, th);    /* This will close the temp file */