decompress to block device using -o is broken

[georgmu]

Describe the bug

When decompressing with "-o" to a block device, the output does not match the source file

To Reproduce
The following script shows the problem:

#!/bin/bash

# via this command, it is possible to specify the zstd binary to use, defaulting to zstd from PATH
ZSTD=${1:-zstd}

# generate our testfile
echo 123456789 > testfile
truncate -s 1M testfile

rm -f testfile.zst
"${ZSTD}" testfile -o testfile.zst

# generate a target file and loop-mount it
dd if=/dev/urandom of=target bs=1M count=1 2>/dev/null

LOOPDEV="$(losetup -f)"
losetup "$LOOPDEV" target

# these are the same, all other sha256 outputs should be the same as well
echo "expected sums:"
sha256sum testfile
"${ZSTD}" -d < testfile.zst | sha256sum

echo

"${ZSTD}" -d testfile.zst -o "$LOOPDEV"
# this one fails
echo
echo "using -o:"
sha256sum "$LOOPDEV"

echo
hexdump -C target | head -n 5
echo
"${ZSTD}" -d < testfile.zst > "$LOOPDEV"
# now it is correct:
echo "using pipe:"
sha256sum "$LOOPDEV"

echo
hexdump -C target | head -n 5

losetup -d "$LOOPDEV"

Sample output:

testfile             :  0.01%   ( 1.000 MiB =>     63 B, testfile.zst)         
expected sums:
a29e830fc0eede27937b8cb0d7336c2c3fcdeedd7c6207923c3e31dec0541537  testfile
a29e830fc0eede27937b8cb0d7336c2c3fcdeedd7c6207923c3e31dec0541537  -

testfile.zst        : 1048576 bytes                                            

using -o:
e074889058caa6266c163e27d1c5990d3810b39927616732102667876c7cefc1  /dev/loop0

00000000  31 32 33 34 35 36 37 38  39 0a 00 00 00 00 00 00  |123456789.......|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00008000  63 4e 0f 16 5c e4 09 2b  a8 0f 3a 55 1e 14 52 a5  |cN..\..+..:U..R.|
00008010  ed 86 59 95 d2 b1 66 93  a4 36 2d 2c 0e 97 b0 a7  |..Y...f..6-,....|

using pipe:
a29e830fc0eede27937b8cb0d7336c2c3fcdeedd7c6207923c3e31dec0541537  /dev/loop0

00000000  31 32 33 34 35 36 37 38  39 0a 00 00 00 00 00 00  |123456789.......|
00000010  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00100000

Expected behavior
-o should generate the same output as piping to a device

Desktop (please complete the following information):

• OS: Linux, Fedaora 38, Debian 11
• tested against dev branch, releases 1.4.8 (debian) and 1.5.4 (fedora)

Additional context
The problem seems to be seeks in the output. This is fine for files, which are generated (seek will generate zeroes in the output file). For block devices, seek must not be used (because it keeps the file contents as is instead of zeroing them as expected)

[georgmu]

doing an strace, it shows that the seeks are in fact the problem:

openat(AT_FDCWD, "/dev/loop0", O_WRONLY|O_CREAT|O_TRUNC, 0600) = 4
fcntl(4, F_GETFL)                       = 0x8001 (flags O_WRONLY|O_LARGEFILE)
newfstatat(4, "", {st_mode=S_IFBLK|0660, st_rdev=makedev(0x7, 0), ...}, AT_EMPTY_PATH) = 0
newfstatat(AT_FDCWD, "/dev/loop0", {st_mode=S_IFBLK|0660, st_rdev=makedev(0x7, 0), ...}, 0) = 0
lseek(4, 0, SEEK_CUR)                   = 0
write(4, "123456789\n\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 32768) = 32768
lseek(4, 1048575, SEEK_SET)             = 1048575
write(4, "\0", 1)                       = 1
close(4)                                = 0

[Cyan4973]

When the -o command was developed, an underlying assumption was that the destination is supposed to be a file.

Using it in combination with other endpoints, such as loop devices, opens a pandora box of problems.

We have 2 choices here :

• extend the territory of -o to other addressable objects, such as loop devices, fix the problems as they get discovered
• restrict -o to files only, refuse to proceed explicitly when it's not.

Option 1 is tempting if the fixes are deemed simple enough, though there is always a risk that more problems will be discovered later.

[georgmu]

I think option 2 is the safest to do. I stumbled across this bug by writing a disk image to a real block device. No error, but broken file system.

I am not familiar with the code and it is pretty complex with regards to multi-OS compatibility. I don't know why this is so complex - maybe due to multi-OS support.

This comment is from my (somewhat naive) linux-only perspective:
I wouldn't make a difference between stdout and filename extraction. Open the file, and operate on that fd, or operate on stdout fd. The only benefit for the file output is seek support (unless there are negative seeks - I don't think they exist in decompression mode). If there would be a flag if seek is possible the seek could be emulated by writing zeroes if seek is not supported. seek support could be disabled for non-regular files.

[Cyan4973]

Work around: on top of using pipes, I think another way to get your use case working is to add the command --no-sparse.

[georgmu]

The commit does not fix the issue.

It writes the message ("Sparse File Support is disabled when output is not a file"), but still uses the lseek syscalls.

[georgmu]

There is an else branch missing which resets sparseFileSupport to 1 (which is ZSTD_SPARSE_DEFAULT):

     if (prefs->sparseFileSupport == 1) {
+        if (!UTIL_isRegularFile(dstFileName)) {
+            prefs->sparseFileSupport = 0;
+            DISPLAYLEVEL(4, "Sparse File Support is disabled when output is not a file \n");
+        }
         prefs->sparseFileSupport = ZSTD_SPARSE_DEFAULT;  // <- This destroys it all again if not in an else branch
     }

Alternatively, the prefs->sparseFileSupport = ZSTD_SPARSE_DEFAULT; could be moved above the check for a regular file or the whole check could be moved into a separate if statement after applying ZSTD_SPARSE_DEFAULT

[Cyan4973]

Thanks for the check and suggestion !

Latest update of dev branch is following your suggestion, and should now better handle the target scenario.