local-overlay store: GC frees nothing under --max-freed, on an uninitialised `bytesFreed`

Open Beginner friendly
#16,269 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
78/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
cpp
Domain
backend

Research direction

Start in src/libstore/gc.cc at collectGarbage's deleteFromStore lambda, then inspect src/libstore/local-overlay-store.cc and tests/functional/local-overlay-store/gc.sh. Run the local-overlay GC test with a finite --max-freed limit and verify lower-only paths contribute zero while upper-layer garbage is collected without an uninitialised-value report.

Written by the indexing model from the issue text.

Description

Describe the bug

LocalOverlayStore::deleteStorePath returns without writing to its bytesFreed out-parameter whenever the path is not present in the upper layer — the whole body is wrapped in if (pathExists(upperPath)). src/libstore/local-overlay-store.cc:

void LocalOverlayStore::deleteStorePath(const std::filesystem::path & path, uint64_t & bytesFreed, bool isKnownPath)
{
    if (path.parent_path() != config->realStoreDir.get()) {
        warn("local-overlay: unexpected gc path %s", PathFmt(path));
        return;                                   // <-- bytesFreed never written
    }

    StorePath storePath = {path.filename().string()};
    auto upperPath = config->toUpperPath(storePath);

    if (pathExists(upperPath)) {
        ...
    }
}                                                 // <-- and here, for lower-only paths

The caller never initialises it. src/libstore/gc.cc, in collectGarbage's deleteFromStore lambda:

uint64_t bytesFreed;
deleteStorePath(realPath, bytesFreed, isKnownPath);

results.bytesFreed += bytesFreed;

if (results.bytesFreed > options.maxFreed) {
    printInfo("deleted more than %d bytes; stopping", options.maxFreed);
    throw GCLimitReached();
}

The only assignment in the chain is deletePath's bytesFreed = 0; in src/libutil/unix/file-system.cc, which neither early return reaches.

So the first dead path that exists in the lower layer and not in the upper layer adds stack garbage to results.bytesFreed. Besides being undefined behaviour, the consequence is a functional one: in practice the garbage value is far above any realistic limit, so GCLimitReached is thrown on that path and the whole collection ends there — including before the upper-layer garbage it could legitimately have deleted. The store then reports its byte target met.

This is not an exotic configuration. Any dead path in the lower layer takes that branch, which is the ordinary state of a local-overlay store, so the first one in readdir order ends the pass. And it is not limited to explicit --max-freed: automatic collection always sets a finite limit, in LocalStore::autoGC:

options.maxFreed = gcSettings.maxFree - avail;

so min-free/max-free collection on a local-overlay store reclaims essentially nothing and the store grows without bound. Only unlimited collections escape, because the default maxFreed is std::numeric_limits<uint64_t>::max() and no accumulated garbage can exceed it. That default is also why the existing test suite does not catch this — see below.

Two details suggest an oversight at a single call site rather than an intended contract:

  • LocalOverlayStore::optimiseStore in the same file already writes uint64_t bytesFreed = 0; before calling the same virtual.
  • deleteStorePath is virtual, so no subclass is obliged to write to the out-parameter, and there is no comment or assertion stating that it must.

Steps To Reproduce

Reproduced with Nix 2.34.8 on Linux 7.1.4. The setup mirrors tests/functional/local-overlay-store/common.sh: an ordinary lower store with registered paths, an overlayfs mount, and some garbage of the overlay store's own in the upper layer.

#!/usr/bin/env bash
# Reproduces: local-overlay GC stops on the first lower-layer path and frees nothing.
set -eu -o pipefail

if [ "${INNER:-}" != 1 ]; then
  exec env INNER=1 unshare --user --map-root-user --mount -- "$0" "$@"
fi

T=$(mktemp -d)
trap 'umount "$T/stores/merged-store/nix/store" 2>/dev/null || true
      umount "$T/stores" 2>/dev/null || true
      rm -rf "$T" 2>/dev/null || true' EXIT

export NIX_CONF_DIR="$T/etc"
mkdir -p "$NIX_CONF_DIR"
printf 'experimental-features = local-overlay-store\nbuild-users-group =\n' > "$NIX_CONF_DIR/nix.conf"

vol="$T/stores"
mkdir -p "$vol"
mount -t tmpfs tmpfs "$vol" 2>/dev/null || true
mkdir -p "$vol"/{store-a/nix/store,store-b,merged-store/nix/store,workdir}

storeA="$vol/store-a"
storeBTop="$vol/store-b"
storeBRoot="$vol/merged-store"
storeB="local-overlay://?root=$storeBRoot&lower-store=$storeA&upper-layer=$storeBTop"

# Lower store: three ordinary registered paths.
for i in 1 2 3; do
  head -c 200000 /dev/urandom > "$T/lower-$i"
  nix-store --store "$storeA" --add "$T/lower-$i" > /dev/null
done

export LIBMOUNT_FORCE_MOUNT2=always
mount -t overlay overlay \
  -o lowerdir="$storeA/nix/store" -o upperdir="$storeBTop" -o workdir="$vol/workdir" \
  "$storeBRoot/nix/store"

# Garbage the overlay store owns and can delete, in the upper layer.
for i in 1 2 3; do
  head -c 300000 /dev/urandom > "$T/upper-$i"
  nix-store --store "$storeB" --add "$T/upper-$i" > /dev/null
done

echo "upper layer size before: $(du -sh "$storeBTop" | cut -f1)"
nix-store --store "$storeB" --gc --max-freed 1000000000000
echo "upper layer size after:  $(du -sh "$storeBTop" | cut -f1)"

Output, with no GC roots anywhere and a limit a million times the size of the whole store:

upper layer before GC:
  hyc4n948phmg68swjqa3z3gxvb53wxdw-upper-1
  q3jxchb7k78sqs8g7bll5cld22f1skd0-upper-2
  xd0b32rb7vllw8h1i9czbm155dklmlkj-upper-3
upper layer size before: 888K

$ nix-store --store "$storeB" --gc --max-freed 1000000000000
finding garbage collector roots...
deleting garbage...
deleting '/nix/store/d3vr392m2c928k3xqbl7j4l9iblajh9p-lower-3'
deleted more than 1000000000000 bytes; stopping
deleting unused links...
note: hard linking is currently saving 0.0 KiB
1 store paths deleted, 85.7 TiB freed

upper layer after GC:
  hyc4n948phmg68swjqa3z3gxvb53wxdw-upper-1
  q3jxchb7k78sqs8g7bll5cld22f1skd0-upper-2
  xd0b32rb7vllw8h1i9czbm155dklmlkj-upper-3
upper layer size after: 888K

same collection with no --max-freed:
6 store paths deleted, 258.8 TiB freed
upper layer after unlimited GC: 0 entries

Nothing was deleted, nothing was freed, and the store reports 85.7 TiB freed out of a store holding roughly 1.5 MB. Removing the limit deletes all three upper-layer paths, so it is the limit check on the garbage value that ends the pass, not any inability to delete. Nine consecutive runs behaved identically, always stopping on the first lower-layer path in readdir order.

The reported total is the one visible signature: a byte count that could not physically have been freed, on a collection that removed nothing.

Because the value read is whatever the stack happens to hold, neither the number nor the early stop is guaranteed by the language. Under valgrind the garbage happened to be small on every run, so the collection completed — while reporting 375.6 MiB freed from the same 1.5 MB store. The uninitialised read itself is reported directly and does not depend on the value:

$ valgrind --track-origins=yes nix-store --store "$storeB" --gc --max-freed 1000000000000
==824568== Conditional jump or move depends on uninitialised value(s)
==824568==    at 0x4EF3C19: nix::LocalStore::collectGarbage(nix::GCOptions const&, nix::GCResults&)
                              ::{lambda(std::basic_string_view<char, ...>, bool)#1}::operator()(...) const
==824568==    by 0x4EFBB7F: nix::LocalStore::collectGarbage(...)::{lambda(nix::StorePath const&)#1}::operator()(...) const
==824568==    by 0x4F607B1: virtual thunk to nix::LocalOverlayStore::collectGarbage(nix::GCOptions const&, nix::GCResults&)
==824568==    by 0x41F1D1E: nix_store::opGC(...)
...
==824568==  Uninitialised value was created by a stack allocation
==824568==    at 0x4EF39E7: nix::LocalStore::collectGarbage(nix::GCOptions const&, nix::GCResults&)
                              ::{lambda(std::basic_string_view<char, ...>, bool)#1}::operator()(...) const

Expected behavior

doc/manual/source/command-ref/nix-store/gc.md documents --max-freed as "Keep deleting paths until at least bytes bytes have been deleted, then stop." A collection should therefore attempt every candidate path and stop only when it has genuinely freed that many bytes. An implementation of deleteStorePath that declines to delete should contribute zero.

Metadata

$ nix-env --version
nix-env (Nix) 2.34.8

Reproduced on Nix 2.34.8. The three source sites are identical in 2.34.7, 2.34.8, and on master at 9137203c1d85c9d13b3d1ef91ba8885b185e5947 (2026-08-06), so this is not fixed in a later release.

LocalOverlayStore::deleteStorePath has had both early returns since the store type was introduced in f0a176e2f1061475546ade22b81343e4b4967568, first released in 2.22.0. The uninitialised declaration in gc.cc is older still.

Additional context

Why the test suite does not catch it

tests/functional/local-overlay-store/gc.sh builds exactly the situation above: after its GC root is removed, the lower store's paths are dead and absent from the upper layer, so the collector does take the uninitialised branch. The test passes because it calls nix-collect-garbage with no limit, and the default maxFreed of UINT64_MAX cannot be exceeded by any accumulated garbage. Its final assertion, that the upper layer is empty, still holds. Passing --max-freed to that same fixture is enough to turn it red.

Where this shows up in practice

We hit this in a sandbox that shares a host store as the lower layer of a guest local-overlay store. There the whole merged store directory consists of lower-layer paths that neither database knows — the GC enumerates the store directory with readdir, so it treats each one as a candidate — and every automatic collection therefore ends on its first candidate. Across two independent boots, each automatic collection attempted exactly one path, freed zero bytes, and reported its byte target met. Space-triggered collection never bounded the store volume, and no error surfaced at any point.

Suggested fix

Initialising at the call site fixes every subclass and every early return at once:

-        uint64_t bytesFreed;
+        uint64_t bytesFreed = 0;
         deleteStorePath(realPath, bytesFreed, isKnownPath);

This is inert on the working path, since deletePath already assigns bytesFreed = 0 on entry.

Built against 2.34.8 and re-run on the reproducer above, it collects the whole store: all six paths attempted, the upper layer emptied, and 6 store paths deleted, 878.9 KiB freed — which is exactly the 3 × 300000 bytes the upper layer held, with the lower-layer paths correctly contributing zero. valgrind --track-origins=yes on the patched build reports no uninitialised value anywhere in collectGarbage; the only remaining reports are Boehm GC's conservative stack scanning, which is present on any Nix run.

Hardening the callee as well would make the contract explicit rather than incidental — assigning bytesFreed = 0 before both early returns in LocalOverlayStore::deleteStorePath, and/or documenting on the virtual that implementations must always write. Happy to open a PR with either shape, whichever maintainers prefer, and to add the --max-freed case to local-overlay-store/gc.sh as a regression test.

One related observation, separable from the above

In the same lambda, printInfo("deleting '%1%'", path) and results.paths.insert(path) both run before the deletion is attempted. On an overlay store that declines to delete a lower-only path, the GC therefore announces and counts as deleted a path it left untouched — the run above reports 1 store paths deleted having deleted none. So results.paths and the reported path count overstate what happened even once the byte count is correct. Mentioned here for context; happy to split it into its own issue if that is preferred.

Checklist

Dominant language
C++
Stars
17.7k
Forks
2k
Avg merge
1d 4h
Merged PRs (30d)
70

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from NixOS/nix

All issues in NixOS/nix

Similar issues

More C++ issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.