bvt's backtrace

Attention: an update is available here.

The Problem

I encountered a vpatch¹ crash when I was trying to press keccak-reground Eucrypt source. There were no hints about possible cause in the logs, so I reached for strace and gdb.

It turned out that the culprit was the mktemp(3) call, which vpatch uses to generate a random, unused path on the filesystem. Musl mktemp(3) requires six placeholder characters in template, but vpatch's template had only three. This code did not crash on GNU systems because glibc relaxes the placeholder characters requirement to three characters, and accepts the template from vpatch without any problems.

The next question was, why didn't vpatch crash when built with ave1's musltronic GNAT 2016? The answer: GNAT Open() function creates a temporary file (marked with Temporary bit) using internal GNAT mechanisms² when its Filename argument is an empty string. Furthermore, GNAT runtime deletes files marked with Temporary bit when they are not used anymore: GNAT 2016 --- during the Finalization phase, GNAT 2017 --- inside Close() call.

Thus, the reconstructed sequence of calls for glibc systems was:

mktemp(3) suceeeds.

GNAT Open() uses the string created by mktemp as the file name.

On Close()/Finalization, GNAT runtime does no special actions -- Temporary bit is not set.

Ada.Directories.Rename() moves the file.

For musl GNAT 2016 systems:

mktemp fails, returning an empty string.

GNAT Open() uses empty string, and creates a file with Temporary bit set.

On Close(), GNAT runtime does no special actions.

Ada.Directories.Rename() moves the file.

During Finalization, GNAT attempts to remove the file, ignores the error.

For musl GNAT 2017 systems:

mktemp fails, returning an empty string.

GNAT Open uses empty string, and creates a file with temporary bit set.

On Close, GNAT runtime deletes the file.

Ada.Directories.Rename() attempts to move the file, throws the exception.

Solutions

Extending the template strings inside vpatch as required by POSIX was my first suggestion. The Lordship has rejected this solution, primarily to reduce the dependence on buggy platform-dependent C interfaces. Instead, asciilifeform suggested a hash-based algorithm for temporary file name generation that uses existing Ada code.

Knowing which algorithm for random filename generation to use, the remaining part to figure out was ensuring that file with the generated name is unused/unique on the file system. There are two ways to do this:

Loop calling stat(2) and then open(2) the file if it does not exist. This solution is inherently racy, requires a source of randomness to function correctly, and even with randomness remains promisetronic. I have decided against it.

Loop trying to open a file 'exclusively'. This is a more reliable solution: it is free from race conditions, and can work without a source of randomness³.

Ada exposes no functions that have 'exclusive open' semantics⁴, so I imported C functions to make this functionality available. I had a choice of using system call wrappers open(2) and close(2), or standard C functions fopen(3) and fclose(3).

I have decided against open(2) because its flags are non-portable:

~/src/musl $ grep -r 'define O_EXCL'
arch/x32/bits/fcntl.h:#define O_EXCL         0200
arch/mips64/bits/fcntl.h:#define O_EXCL        02000
arch/aarch64/bits/fcntl.h:#define O_EXCL         0200
arch/arm/bits/fcntl.h:#define O_EXCL         0200
arch/powerpc/bits/fcntl.h:#define O_EXCL         0200
arch/generic/bits/fcntl.h:#define O_EXCL         0200
arch/x86_64/bits/fcntl.h:#define O_EXCL         0200
arch/mips/bits/fcntl.h:#define O_EXCL        02000
arch/mipsn32/bits/fcntl.h:#define O_EXCL        02000
arch/m68k/bits/fcntl.h:#define O_EXCL         0200
arch/s390x/bits/fcntl.h:#define O_EXCL         0200
arch/powerpc64/bits/fcntl.h:#define O_EXCL         0200
~/src/musl $ grep -r 'define O_CREAT'
arch/x32/bits/fcntl.h:#define O_CREAT        0100
arch/mips64/bits/fcntl.h:#define O_CREAT        0400
arch/aarch64/bits/fcntl.h:#define O_CREAT        0100
arch/arm/bits/fcntl.h:#define O_CREAT        0100
arch/powerpc/bits/fcntl.h:#define O_CREAT        0100
arch/generic/bits/fcntl.h:#define O_CREAT        0100
arch/x86_64/bits/fcntl.h:#define O_CREAT        0100
arch/mips/bits/fcntl.h:#define O_CREAT        0400
arch/mipsn32/bits/fcntl.h:#define O_CREAT        0400
arch/m68k/bits/fcntl.h:#define O_CREAT        0100
arch/s390x/bits/fcntl.h:#define O_CREAT        0100
arch/powerpc64/bits/fcntl.h:#define O_CREAT        0100

Handling Linux portability problems would have created a whole new level of complexity for vpatch.

fopen(3) has "x" mode specifier for exclusive open. This mode specifier is available on most OSes, but it is an extension rather than standard functionality. Also, fopen(3) creates files with 0666 mode (after applying the umask it typically becomes 0644). Another drawback of fopen is that is does more work: it mallocs buffers and checks whether the opened file is a terminal. It is also documented to fail on certain file systems-kernel version combinations⁵.

You may want to discuss the following points:

Temporary files are now created in /tmp, because /tmp on NFS is extremely unlikely. This also required switching from Rename() to Copy_File(). Rename() on POSIX platforms is a thin wrapper around rename(2), which does not work across file systems.

There is no error handling for fopen loop. Thus, vpatch will loop indefinitely if /tmp partition is full. The problem here is that getting errno is impossible without extremely ugly hacks, adding C code to vtools, or importing GNAT-specific C code (GNAT/System.OS_Lib.Errno), and I would like to avoid all of these options.

The vpatches offered below do not use Interfaces.C.Strings, as ave1 and mircea_popescu suggested.

Vpatches

First of all, this code is built on top of the vtools_ksum.vpatch, which is not included in the latest release, though is present on btcbase repository.

curl 'http://btcbase.org/data/vtools/vtools_ksum.vpatch' > vtools_ksum.vpatch
curl 'http://btcbase.org/data/vtools/vtools_ksum.vpatch.phf.sig' > seals/vtools_ksum.vpatch.phf.sig

Then, the keccak patches, which will be useful only after the vtools tree is reground:

curl 'http://bvt-trace.net/vpatches/keccak-vtools_tempfile_standalone.vpatch' > keccak-vtools_tempfile_standalone.vpatch
curl 'http://bvt-trace.net/vpatches/keccak-vtools_tempfile_standalone.vpatch.bvt.sig' > keccak-vtools_tempfile_standalone.vpatch.bvt.sig

Finally, so that you can press this vpatch on top of vtools today, the sha512 vpatches:

curl 'http://bvt-trace.net/vpatches/vtools_tempfile_standalone.vpatch' > vtools_tempfile_standalone.vpatch
curl 'http://bvt-trace.net/vpatches/vtools_tempfile_standalone.vpatch.bvt.sig' > vtools_tempfile_standalone.vpatch.bvt.sig

You may need my public key:

curl 'http://wot.deedbot.org/6CF3EFF892A7F23E7E798E5EBA6B8C054B962B68.asc' > bvt.asc

Please press, test, and comment.

1. Static binary, GNAT 2017, musl-libc. [↩]
2. Which fall back to C, which is common in GNAT runtime. [↩]
3. Entropy source would still improve the performance of vpatch under heavy file creation contention. I believe this is not that important case for optimizations. [↩]
4. Later, ave1 did found such a function in GNAT-specific package. [↩]
5. 'On NFS, O_EXCL is supported only when using NFSv3 or later on kernel 2.6 or later.' [↩]