From aeb810b01e17d040f9592681ee271f15874dce50 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Jul 2013 14:01:25 +0200 Subject: [PATCH] Garbage collector: Don't follow symlinks arbitrarily Only indirect roots (symlinks to symlinks to the Nix store) are now supported. --- Makefile.am | 2 - src/libstore/gc.cc | 82 +++++++++++++++++++-------------------- src/libstore/store-api.cc | 5 +-- src/libutil/util.cc | 9 +++++ src/libutil/util.hh | 4 ++ 5 files changed, 54 insertions(+), 48 deletions(-) diff --git a/Makefile.am b/Makefile.am index 9705e22090..d3ef224de3 100644 --- a/Makefile.am +++ b/Makefile.am @@ -27,11 +27,9 @@ init-state: $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/profiles $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/gcroots $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/temproots - ln -sfn $(localstatedir)/nix/profiles $(DESTDIR)$(localstatedir)/nix/gcroots/profiles $(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(localstatedir)/nix/userpool -$(INSTALL) $(INIT_FLAGS) -d $(DESTDIR)$(storedir) $(INSTALL) $(INIT_FLAGS) $(GROUP_WRITABLE) -d $(DESTDIR)$(localstatedir)/nix/manifests - ln -sfn $(localstatedir)/nix/manifests $(DESTDIR)$(localstatedir)/nix/gcroots/manifests else diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 5c5c07e4c4..37ca6be4b8 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -258,8 +258,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds) only succeed if the owning process has died. In that case we don't care about its temporary roots. */ if (lockFile(*fd, ltWrite, false)) { - printMsg(lvlError, format("removing stale temporary roots file `%1%'") - % path); + printMsg(lvlError, format("removing stale temporary roots file `%1%'") % path); unlink(path.c_str()); writeFull(*fd, (const unsigned char *) "d", 1); continue; @@ -290,46 +289,47 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds) } -static void findRoots(StoreAPI & store, const Path & path, - bool recurseSymlinks, bool deleteStale, Roots & roots) +static void foundRoot(StoreAPI & store, + const Path & path, const Path & target, Roots & roots) +{ + Path storePath = toStorePath(target); + if (store.isValidPath(storePath)) + roots[path] = storePath; + else + printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") % path % storePath); +} + + +static void findRoots(StoreAPI & store, const Path & path, Roots & roots) { try { - struct stat st; - if (lstat(path.c_str(), &st) == -1) - throw SysError(format("statting `%1%'") % path); - - printMsg(lvlVomit, format("looking at `%1%'") % path); + struct stat st = lstat(path); if (S_ISDIR(st.st_mode)) { Strings names = readDirectory(path); foreach (Strings::iterator, i, names) - findRoots(store, path + "/" + *i, recurseSymlinks, deleteStale, roots); + findRoots(store, path + "/" + *i, roots); } else if (S_ISLNK(st.st_mode)) { - Path target = absPath(readLink(path), dirOf(path)); + Path target = readLink(path); + if (isInStore(target)) + foundRoot(store, path, target, roots); - if (isInStore(target)) { - debug(format("found root `%1%' in `%2%'") - % target % path); - Path storePath = toStorePath(target); - if (store.isValidPath(storePath)) - roots[path] = storePath; - else - printMsg(lvlInfo, format("skipping invalid root from `%1%' to `%2%'") - % path % storePath); - } - - else if (recurseSymlinks) { - if (pathExists(target)) - findRoots(store, target, false, deleteStale, roots); - else if (deleteStale) { - printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target); - /* Note that we only delete when recursing, i.e., - when we are still in the `gcroots' tree. We - never delete stuff outside that tree. */ - unlink(path.c_str()); + /* Handle indirect roots. */ + else { + target = absPath(target, dirOf(path)); + if (!pathExists(target)) { + if (isInDir(path, settings.nixStateDir + "/" + gcRootsDir + "/auto")) { + printMsg(lvlInfo, format("removing stale link from `%1%' to `%2%'") % path % target); + unlink(path.c_str()); + } + } else { + struct stat st2 = lstat(target); + if (!S_ISLNK(st2.st_mode)) return; + Path target2 = readLink(target); + if (isInStore(target2)) foundRoot(store, path, target2, roots); } } } @@ -346,18 +346,16 @@ static void findRoots(StoreAPI & store, const Path & path, } -static Roots findRoots(StoreAPI & store, bool deleteStale) -{ - Roots roots; - Path rootsDir = canonPath((format("%1%/%2%") % settings.nixStateDir % gcRootsDir).str()); - findRoots(store, rootsDir, true, deleteStale, roots); - return roots; -} - - Roots LocalStore::findRoots() { - return nix::findRoots(*this, false); + Roots roots; + + /* Process direct roots in {gcroots,manifests,profiles}. */ + nix::findRoots(*this, settings.nixStateDir + "/" + gcRootsDir, roots); + nix::findRoots(*this, settings.nixStateDir + "/manifests", roots); + nix::findRoots(*this, settings.nixStateDir + "/profiles", roots); + + return roots; } @@ -637,7 +635,7 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) /* Find the roots. Since we've grabbed the GC lock, the set of permanent roots cannot increase now. */ printMsg(lvlError, format("finding garbage collector roots...")); - Roots rootMap = options.ignoreLiveness ? Roots() : nix::findRoots(*this, true); + Roots rootMap = options.ignoreLiveness ? Roots() : findRoots(); foreach (Roots::iterator, i, rootMap) state.roots.insert(i->second); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 32aaca6be0..0f250a3c7c 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -18,10 +18,7 @@ GCOptions::GCOptions() bool isInStore(const Path & path) { - return path[0] == '/' - && string(path, 0, settings.nixStore.size()) == settings.nixStore - && path.size() >= settings.nixStore.size() + 2 - && path[settings.nixStore.size()] == '/'; + return isInDir(path, settings.nixStore); } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index b1231e4557..dad5f624bd 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -148,6 +148,15 @@ string baseNameOf(const Path & path) } +bool isInDir(const Path & path, const Path & dir) +{ + return path[0] == '/' + && string(path, 0, dir.size()) == dir + && path.size() >= dir.size() + 2 + && path[dir.size()] == '/'; +} + + struct stat lstat(const Path & path) { struct stat st; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 9caab4aa10..6c83987c5d 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -45,6 +45,10 @@ Path dirOf(const Path & path); following the final `/'. */ string baseNameOf(const Path & path); +/* Check whether a given path is a descendant of the given + directory. */ +bool isInDir(const Path & path, const Path & dir); + /* Get status of `path'. */ struct stat lstat(const Path & path);