From f40fdaa43271edf98b80c08e20f401b5da591501 Mon Sep 17 00:00:00 2001 From: Julian Andres Klode Date: Sat, 19 Mar 2016 01:56:38 +0100 Subject: cachefile: Only set members that were initialized successfully Otherwise, things will just start failing later down the stack, because (a) the lazy getters do not check if building was successful and (b) any further getter call would return the invalid object anyway. Also initialize VS in pkgCache to nullptr by default. Closes: #818628 --- apt-pkg/cachefile.cc | 62 ++++++++++++++++------ apt-pkg/pkgcache.cc | 2 +- test/integration/test-bug-818628-unreadable-source | 42 +++++++++++++++ 3 files changed, 89 insertions(+), 17 deletions(-) create mode 100755 test/integration/test-bug-818628-unreadable-source diff --git a/apt-pkg/cachefile.cc b/apt-pkg/cachefile.cc index 909da5cb4..0e2192973 100644 --- a/apt-pkg/cachefile.cc +++ b/apt-pkg/cachefile.cc @@ -32,6 +32,7 @@ #include #include #include +#include #include /*}}}*/ @@ -69,9 +70,13 @@ public: ScopedErrorMerge() { _error->PushToStack(); } ~ScopedErrorMerge() { _error->MergeWithStack(); } }; + bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock) { - if (Cache != NULL) + std::unique_ptr Cache; + std::unique_ptr Map; + + if (this->Cache != NULL) return true; ScopedErrorMerge sem; @@ -80,11 +85,16 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock) FileFd file(_config->FindFile("Dir::Cache::pkgcache"), FileFd::ReadOnly); if (file.IsOpen() == false || file.Failed()) return false; - Map = new MMap(file, MMap::Public|MMap::ReadOnly); + Map.reset(new MMap(file, MMap::Public|MMap::ReadOnly)); if (unlikely(Map->validData() == false)) return false; - Cache = new pkgCache(Map); - return _error->PendingError() == false; + Cache.reset(new pkgCache(Map.get())); + if (_error->PendingError() == true) + return false; + + this->Cache = Cache.release(); + this->Map = Map.release(); + return true; } if (WithLock == true) @@ -94,11 +104,15 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock) if (_error->PendingError() == true) return false; - BuildSourceList(Progress); + if (BuildSourceList(Progress) == false) + return false; // Read the caches - Cache = nullptr; - bool Res = pkgCacheGenerator::MakeStatusCache(*SrcList,Progress,&Map, &Cache, true); + MMap *TmpMap = nullptr; + pkgCache *TmpCache = nullptr; + bool Res = pkgCacheGenerator::MakeStatusCache(*SrcList,Progress,&TmpMap, &TmpCache, true); + Map.reset(TmpMap); + Cache.reset(TmpCache); if (Progress != NULL) Progress->Done(); if (Res == false) @@ -109,9 +123,12 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock) _error->Warning(_("You may want to run apt-get update to correct these problems")); if (Cache == nullptr) - Cache = new pkgCache(Map); + Cache.reset(new pkgCache(Map.get())); if (_error->PendingError() == true) return false; + this->Map = Map.release(); + this->Cache = Cache.release(); + return true; } /*}}}*/ @@ -120,12 +137,14 @@ bool pkgCacheFile::BuildCaches(OpProgress *Progress, bool WithLock) /* */ bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/) { - if (SrcList != NULL) + std::unique_ptr SrcList; + if (this->SrcList != NULL) return true; - SrcList = new pkgSourceList(); + SrcList.reset(new pkgSourceList()); if (SrcList->ReadMainList() == false) return _error->Error(_("The list of sources could not be read.")); + this->SrcList = SrcList.release(); return true; } /*}}}*/ @@ -134,16 +153,18 @@ bool pkgCacheFile::BuildSourceList(OpProgress * /*Progress*/) /* */ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/) { - if (Policy != NULL) + std::unique_ptr Policy; + if (this->Policy != NULL) return true; - Policy = new pkgPolicy(Cache); + Policy.reset(new pkgPolicy(Cache)); if (_error->PendingError() == true) return false; if (ReadPinFile(*Policy) == false || ReadPinDir(*Policy) == false) return false; + this->Policy = Policy.release(); return true; } /*}}}*/ @@ -152,17 +173,21 @@ bool pkgCacheFile::BuildPolicy(OpProgress * /*Progress*/) /* */ bool pkgCacheFile::BuildDepCache(OpProgress *Progress) { - if (DCache != NULL) + std::unique_ptr DCache; + if (this->DCache != NULL) return true; if (BuildPolicy(Progress) == false) return false; - DCache = new pkgDepCache(Cache,Policy); + DCache.reset(new pkgDepCache(Cache,Policy)); if (_error->PendingError() == true) return false; + if (DCache->Init(Progress) == false) + return false; - return DCache->Init(Progress); + this->DCache = DCache.release(); + return true; } /*}}}*/ // CacheFile::Open - Open the cache files, creating if necessary /*{{{*/ @@ -225,7 +250,12 @@ bool pkgCacheFile::AddIndexFile(pkgIndexFile * const File) /*{{{*/ return false; } Cache = new pkgCache(Map); - return _error->PendingError() == false; + if (_error->PendingError() == true) { + delete Cache; + Cache = nullptr; + return false; + } + return true; } else { diff --git a/apt-pkg/pkgcache.cc b/apt-pkg/pkgcache.cc index 159b5e00e..83cb6149e 100644 --- a/apt-pkg/pkgcache.cc +++ b/apt-pkg/pkgcache.cc @@ -127,7 +127,7 @@ bool pkgCache::Header::CheckSizes(Header &Against) const // --------------------------------------------------------------------- /* */ APT_IGNORE_DEPRECATED_PUSH -pkgCache::pkgCache(MMap *Map, bool DoMap) : Map(*Map), d(NULL) +pkgCache::pkgCache(MMap *Map, bool DoMap) : Map(*Map), VS(nullptr), d(NULL) { // call getArchitectures() with cached=false to ensure that the // architectures cache is re-evaulated. this is needed in cases diff --git a/test/integration/test-bug-818628-unreadable-source b/test/integration/test-bug-818628-unreadable-source new file mode 100755 index 000000000..84776dd8d --- /dev/null +++ b/test/integration/test-bug-818628-unreadable-source @@ -0,0 +1,42 @@ +#!/bin/sh +set -e + +TESTDIR="$(readlink -f "$(dirname "$0")")" +. "$TESTDIR/framework" +setupenvironment +configarchitecture 'amd64' 'i386' + +if [ "$(id -u)" = "0" ]; then + msgskip "Tests for unreadable sources.list do not work as root" + exit 0 +fi + +insertinstalledpackage 'apt' 'i386' '1' +buildsimplenativepackage 'apt' 'i386' '2' 'unstable' + +setupaptarchive + +# Test unreadable sources.list files +chmod -r rootdir/etc/apt/sources.list.d + +testfailureequal "E: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied) +W: You may want to run apt-get update to correct these problems +E: The package cache file is corrupted" aptcache policy apt +testfailureequal "E: Unable to read $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/ - opendir (13: Permission denied) +W: You may want to run apt-get update to correct these problems +E: The package cache file is corrupted" apt search apt + +chmod +r rootdir/etc/apt/sources.list.d + + +# Test unreadable sources.list files +chmod -r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list + +testfailureequal "E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied) +E: The list of sources could not be read. +E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied) +E: The list of sources could not be read." aptcache policy apt +testfailureequal "E: Opening $TMPWORKINGDIRECTORY/rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list - ifstream::ifstream (13: Permission denied) +E: The list of sources could not be read." apt search apt + +chmod +r rootdir/etc/apt/sources.list.d/apt-test-unstable-deb-src.list -- cgit v1.2.3-70-g09d2