util: Prevent file overwriting in fallback `AllocateFileRange` implementation #35524

pull winterrdog wants to merge 2 commits into bitcoin:master from winterrdog:fix/posix-fallback-alloc changing 1 files +26 −13

winterrdog commented at 10:12 AM on June 13, 2026: contributor

this PR picks up #33164

on systems where posix_fallocate() is unavailable, the fallback implementation of AllocateFileRange() can overwrite parts of blk00000.dat with zeroes. as a result, -reindex appears to complete successfully, but only the genesis block is recovered

this PR fixes the fallback path so that when reindexing happens, the existing blocks are correctly restored instead of getting clobbered

the second commit (fdec41914f2218c3c8bfe033a2612c6029854a1a) takes some inspiration from #33228, specifically this commit 2c113ebc2d3ab559ccaf400a1953724bb15b1574.

fixes #33128

util: Check file size before extending on Windows

The Windows implementation of `AllocateFileRange` previously attempted
to extend the file unconditionally, regardless of its current size.

Now, it reads the current file size first and only extends when the requested
range exceeds the existing end of the file.

fdec41914f

DrahtBot added the label Utils/log/libs on Jun 13, 2026
DrahtBot commented at 10:12 AM on June 13, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35524.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #33228 (util: Address various issues in AllocateFileRange by luke-jr)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

winterrdog commented at 10:15 AM on June 13, 2026: contributor

test plan

bug reproduction (before the fix):

on an affected system (e.g. OpenBSD or NetBSD), follow the steps in the issue's description.

on systems that have posix_fallocate(), force the fallback path by changing (profusely based on maflcko's comment):

diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp
index be7f1ee5a2..ada3280dbc 100644
--- a/src/util/fs_helpers.cpp
+++ b/src/util/fs_helpers.cpp
@@ -200,7 +200,7 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
     }
     ftruncate(fileno(file), static_cast<off_t>(offset) + length);
 #else
-#if defined(HAVE_POSIX_FALLOCATE)
+#if 0
     // Version using posix_fallocate
     off_t nEndPos = (off_t)offset + length;
     if (0 == posix_fallocate(fileno(file), 0, nEndPos)) return;

then run:

./build/test/functional/test_runner.py ./build/test/functional/feature_reindex_init.py --failfast

which MUST fail with: AssertionError: not(0 == 200).

fix verification (after applying the fix)

after reindexing, getblockcount should return the same number of blocks that existed before deleting the index/ directory.

cc: @luke-jr @cedwies @hebasto

in src/util/fs_helpers.cpp:227 in 88fd83ecf0

 222 | @@ -216,16 +223,22 @@ void AllocateFileRange(FILE* file, unsigned int offset, unsigned int length)
 223 |  #endif
 224 |      // Fallback version
 225 |      // TODO: just write one byte per block
 226 | -    static const char buf[65536] = {};
 227 | -    if (fseek(file, offset, SEEK_SET)) {
 228 | -        return;
 229 | +    if (fseeko(file, 0 , SEEK_END) != 0) {
 230 | +      return;

thomasbuilds commented at 12:02 PM on June 13, 2026:

wrong indentation

winterrdog commented at 7:06 PM on June 13, 2026:

thanks! fixed

winterrdog force-pushed on Jun 13, 2026
in src/util/fs_helpers.cpp:233 in b07e08c6d8
```
 236 | -        length -= now;
 237 | +    off_t file_size{ftello(file)};
 238 | +    if (file_size < 0) {
 239 | +        return;
 240 | +    }
 241 | +    off_t end_pos{offset + length};
```
thomasbuilds commented at 7:37 AM on June 14, 2026:

nit: the Windows, macOS, and posix computations above all widen offset before adding length

off_t end_pos{static_cast<off_t>(offset) + length};

what do you think?
winterrdog commented at 6:14 PM on June 16, 2026:

i think it's a good point for safety.

since blk*.dat files are capped (the allocation that later does the pre-allocation can only happen if the assertion passed) at 128 MiB, overflow is not an issue here, but explicitly casting offset makes the code more uniform to match the the other variants

applied the change

util: Fix data clobbering in fallback AllocateFileRange

The fallback variant of `AllocateFileRange` (introduced in #1677)
unconditionally wrote zeros into the file regardless of its current
size. During `-reindex`, this could overwrite existing block data in
`blk*.dat` files before the reindex scan ran. As a result, `-reindex`
was broken on platforms using the fallback implementation, including
*BSDs, OmniOS, and OpenIndiana. After reindexing, `getblockcount`
would return 0 because the block data had already been overwritten.

The fix? Only zero-fill the portion of the requested range that
extends beyond the current end of the file.

Fixes #33128

65b44fcf0b

winterrdog force-pushed on Jun 16, 2026
in src/util/fs_helpers.cpp:222 in 65b44fcf0b

winterrdog commented at 6:59 PM on June 16, 2026:

was thinking about making this an append-only operation in this PR, but before I go ahead I need to get @sedited's thoughts (because of your comment on this commit). what led me to think about it is that, when I was looking at the only call site, it suggests the pre-allocation is fundamentally an append-only operation (i.e. always extending from the current file end in chunk-aligned steps).

just want to understand the intended contract here a bit better before changing anything. under what circumstances would AllocateFileRange() ever be called with an offset that is not equal to the current end of file (i.e. a non-contiguous region / hole in the file) ?

Contributors

Labels

Linked (view graph)

#33128 *BSD, OmniOS: `-reindex` is broken #33164 Avoid file overwriting in fallback `AllocateFileRange` implementation #33228 util: Address various issues in AllocateFileRange

util: Prevent file overwriting in fallback `AllocateFileRange` implementation #35524

Code Coverage & Benchmarks

Reviews

Conflicts

test plan

bug reproduction (before the fix):

fix verification (after applying the fix)