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 1f5e6d270230d90feaf29cf551eb952b6163a5cc..749886877c84e95639c0199345e9e34d53c5b556 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1262,10 +1262,11 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
 #ifdef HAVE_LIBZ
    if (state.ztarfile != NULL)
    {
+       errno = 0;              /* in case gzclose() doesn't set it */
        if (gzclose(state.ztarfile) != 0)
        {
-           pg_log_error("could not close compressed file \"%s\": %s",
-                        state.filename, get_gz_error(state.ztarfile));
+           pg_log_error("could not close compressed file \"%s\": %m",
+                        state.filename);
            exit(1);
        }
    }

diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c
index 646bfd1e266872c7444195190296f4785bb84951..223d0a05b08ebdde55c0ada4bfd0221763b95558 100644 (file)
--- a/src/bin/pg_basebackup/receivelog.c
+++ b/src/bin/pg_basebackup/receivelog.c
@@ -71,7 +71,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 2fc6b46f59bc7a7fb2f6b3b94760bc715eab0bb0..d1f0a8a2743be57d1f7f791255070e8aaabde508 100644 (file)
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -235,7 +235,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 10423dc726bc13ef35b242ebeca00f798f5875b3..63df3e56e74fd579b0cc8ec7e5fcf06128631260 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -268,6 +268,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)
@@ -1578,6 +1579,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 67f5df22af687fb1dc71599e25b8ec1dc3bc00a4..2398af265bbf1ffb8a8ffbda6d048644956b25c4 100644 (file)
--- a/src/bin/pg_dump/pg_backup_directory.c
+++ b/src/bin/pg_dump/pg_backup_directory.c
@@ -369,7 +369,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;
 }
@@ -680,7 +681,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 */
@@ -699,7 +701,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 2aee13848ee7818db3f3276fd6bf1451745b6900..edba568309d497bb55083d2dcb860dac30357577 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 */