Merge pull request #3541 from felixhandte/fix-setvbuf-segfault

Avoid Segfault Caused by Calling `setvbuf()` on Null File Pointer

Author: felixhandte

programs/fileio.c

@@ -649,18 +649,24 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs,
 #endif
         if (f == NULL) {
             DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno));
+        } else {
+            /* An increased buffer size can provide a significant performance
+             * boost on some platforms. Note that providing a NULL buf with a
+             * size that's not 0 is not defined in ANSI C, but is defined in an
+             * extension. There are three possibilities here:
+             * 1. Libc supports the extended version and everything is good.
+             * 2. Libc ignores the size when buf is NULL, in which case
+             *    everything will continue as if we didn't call `setvbuf()`.
+             * 3. We fail the call and execution continues but a warning
+             *    message might be shown.
+             * In all cases due execution continues. For now, I believe that
+             * this is a more cost-effective solution than managing the buffers
+             * allocations ourselves (will require an API change).
+             */
+            if (setvbuf(f, NULL, _IOFBF, 1 MB)) {
+                DISPLAYLEVEL(2, "Warning: setvbuf failed for %s\n", dstFileName);
+            }
         }
-        /* An increased buffer size can provide a significant performance boost on some platforms.
-         * Note that providing a NULL buf with a size that's not 0 is not defined in ANSI C, but is defined
-         * in an extension. There are three possibilities here -
-         * 1. Libc supports the extended version and everything is good.
-         * 2. Libc ignores the size when buf is NULL, in which case everything will continue as if we didn't
-         *    call `setvbuf`.
-         * 3. We fail the call and execution continues but a warning message might be shown.
-         * In all cases due execution continues. For now, I believe that this is a more cost-effective
-         * solution than managing the buffers allocations ourselves (will require an API change). */
-        if(setvbuf(f, NULL, _IOFBF, 1 MB))
-            DISPLAYLEVEL(2, "Warning: setvbuf failed for %s\n", dstFileName);
         return f;
     }
 }

tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+
+# motivated by issue #3523
+
+datagen > file
+mkdir out
+chmod 000 out
+
+zstd file -q --trace-file-stat -o out/file.zst
+zstd -tq out/file.zst
+
+chmod 777 out

tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact

@@ -0,0 +1,26 @@
+Trace:FileStat: > UTIL_isLink(file)
+Trace:FileStat: < 0
+Trace:FileStat: > UTIL_isConsole(2)
+Trace:FileStat: < 0
+Trace:FileStat: > UTIL_getFileSize(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
+Trace:FileStat:  < 1
+Trace:FileStat: < 65537
+Trace:FileStat: > UTIL_stat(-1, file)
+Trace:FileStat: < 1
+Trace:FileStat: > UTIL_isDirectoryStat()
+Trace:FileStat: < 0
+Trace:FileStat: > UTIL_stat(-1, file)
+Trace:FileStat: < 1
+Trace:FileStat: > UTIL_isSameFile(file, out/file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file)
+Trace:FileStat:  < 1
+Trace:FileStat:  > UTIL_stat(-1, out/file.zst)
+Trace:FileStat:  < 0
+Trace:FileStat: < 0
+Trace:FileStat: > UTIL_isRegularFile(out/file.zst)
+Trace:FileStat:  > UTIL_stat(-1, out/file.zst)
+Trace:FileStat:  < 0
+Trace:FileStat: < 0
+zstd: out/file.zst: Permission denied
+zstd: can't stat out/file.zst : Permission denied -- ignored 

tests/cli-tests/run.py

@@ -535,7 +535,8 @@ def _run_script(self, script: str, cwd: str) -> None:
             subprocess.run(
                 args=[script],
                 stdin=subprocess.DEVNULL,
-                capture_output=True,
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
                 cwd=cwd,
                 env=env,
                 check=True,