Avoid leaking duplicated file descriptors in corner cases.

pg_dump's compression modules had variations on the theme of

	fp = fdopen(dup(fd), mode);
	if (fp == NULL)
	    // fail, reporting errno

which is problematic for two reasons.  First, if dup() succeeds but
fdopen() fails, we'd leak the duplicated FD.  That's not important
at present since the program will just exit immediately after failure
anyway; but perhaps someday we'll try to continue, making the resource
leak potentially significant.  Second, if dup() fails then fdopen()
will overwrite the useful errno (perhaps EMFILE) with a misleading
value EBADF, making it difficult to understand what went wrong.
Fix both issues by testing for dup() failure before proceeding to
the next call.

These failures are sufficiently unlikely, and the consequences minor
enough, that this doesn't seem worth the effort to back-patch.
But let's fix it in HEAD.

Author: Jianghua Yang <yjhjstz@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/62bbe34d-2315-4b42-b768-56d901aa83e1@gmail.com

diff --git a/src/bin/pg_dump/compress_gzip.c b/src/bin/pg_dump/compress_gzip.c
index c9ce8a53aaaf9fcb4dfd8858f86cc02fcd7311c9..60c553ba25ada1b9a81412d34b17e5ce1da754d4 100644 (file)
--- a/src/bin/pg_dump/compress_gzip.c
+++ b/src/bin/pg_dump/compress_gzip.c
@@ -386,12 +386,24 @@ Gzip_open(const char *path, int fd, const char *mode, CompressFileHandle *CFH)
                strcpy(mode_compression, mode);
 
        if (fd >= 0)
-               gzfp = gzdopen(dup(fd), mode_compression);
+       {
+               int                     dup_fd = dup(fd);
+
+               if (dup_fd < 0)
+                       return false;
+               gzfp = gzdopen(dup_fd, mode_compression);
+               if (gzfp == NULL)
+               {
+                       close(dup_fd);
+                       return false;
+               }
+       }
        else
+       {
                gzfp = gzopen(path, mode_compression);
-
-       if (gzfp == NULL)
-               return false;
+               if (gzfp == NULL)
+                       return false;
+       }
 
        CFH->private_data = gzfp;
 

diff --git a/src/bin/pg_dump/compress_lz4.c b/src/bin/pg_dump/compress_lz4.c
index 0aa519fbb6741c621a9669df80d73793246e325d..0a7872116e76327e65dfe360daad1e6526826f7f 100644 (file)
--- a/src/bin/pg_dump/compress_lz4.c
+++ b/src/bin/pg_dump/compress_lz4.c
@@ -715,13 +715,30 @@ LZ4Stream_open(const char *path, int fd, const char *mode,
        LZ4State   *state = (LZ4State *) CFH->private_data;
 
        if (fd >= 0)
-               state->fp = fdopen(dup(fd), mode);
+       {
+               int                     dup_fd = dup(fd);
+
+               if (dup_fd < 0)
+               {
+                       state->errcode = errno;
+                       return false;
+               }
+               state->fp = fdopen(dup_fd, mode);
+               if (state->fp == NULL)
+               {
+                       state->errcode = errno;
+                       close(dup_fd);
+                       return false;
+               }
+       }
        else
-               state->fp = fopen(path, mode);
-       if (state->fp == NULL)
        {
-               state->errcode = errno;
-               return false;
+               state->fp = fopen(path, mode);
+               if (state->fp == NULL)
+               {
+                       state->errcode = errno;
+                       return false;
+               }
        }
 
        return true;

diff --git a/src/bin/pg_dump/compress_none.c b/src/bin/pg_dump/compress_none.c
index d862d8ca6e960c96fc913e0530850ac012e3e0d2..743e2ce94b55b9f916c8c3d3ff41e69a492e293f 100644 (file)
--- a/src/bin/pg_dump/compress_none.c
+++ b/src/bin/pg_dump/compress_none.c
@@ -231,12 +231,24 @@ open_none(const char *path, int fd, const char *mode, CompressFileHandle *CFH)
        Assert(CFH->private_data == NULL);
 
        if (fd >= 0)
-               CFH->private_data = fdopen(dup(fd), mode);
+       {
+               int                     dup_fd = dup(fd);
+
+               if (dup_fd < 0)
+                       return false;
+               CFH->private_data = fdopen(dup_fd, mode);
+               if (CFH->private_data == NULL)
+               {
+                       close(dup_fd);
+                       return false;
+               }
+       }
        else
+       {
                CFH->private_data = fopen(path, mode);
-
-       if (CFH->private_data == NULL)
-               return false;
+               if (CFH->private_data == NULL)
+                       return false;
+       }
 
        return true;
 }

diff --git a/src/bin/pg_dump/compress_zstd.c b/src/bin/pg_dump/compress_zstd.c
index cf2db2649ac07f1f6325d1b92680822a02e29a42..68f1d8159171ffddccbb6f991b59c25dd86e97ce 100644 (file)
--- a/src/bin/pg_dump/compress_zstd.c
+++ b/src/bin/pg_dump/compress_zstd.c
@@ -523,14 +523,30 @@ Zstd_open(const char *path, int fd, const char *mode,
        }
 
        if (fd >= 0)
-               fp = fdopen(dup(fd), mode);
-       else
-               fp = fopen(path, mode);
+       {
+               int                     dup_fd = dup(fd);
 
-       if (fp == NULL)
+               if (dup_fd < 0)
+               {
+                       pg_free(zstdcs);
+                       return false;
+               }
+               fp = fdopen(dup_fd, mode);
+               if (fp == NULL)
+               {
+                       close(dup_fd);
+                       pg_free(zstdcs);
+                       return false;
+               }
+       }
+       else
        {
-               pg_free(zstdcs);
-               return false;
+               fp = fopen(path, mode);
+               if (fp == NULL)
+               {
+                       pg_free(zstdcs);
+                       return false;
+               }
        }
 
        zstdcs->fp = fp;