Attention: an update is available here.
The Problem
I encountered a vpatch1 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 mechanisms2 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:
For musl GNAT 2016 systems:
For musl GNAT 2017 systems:
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:
Ada exposes no functions that have 'exclusive open' semantics4, 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 combinations5.
You may want to discuss the following points:
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.
- Static binary, GNAT 2017, musl-libc. [↩]
- Which fall back to C, which is common in GNAT runtime. [↩]
- Entropy source would still improve the performance of vpatch under heavy file creation contention. I believe this is not that important case for optimizations. [↩]
- Later, ave1 did found such a function in GNAT-specific package. [↩]
- 'On NFS, O_EXCL is supported only when using NFSv3 or later on kernel 2.6 or later.' [↩]
Log thread, for future ref:
http://btcbase.org/log/2018-10-24#1865524