From: David Kalnischkies Date: Tue, 21 Jul 2009 14:54:28 +0000 (+0200) Subject: eliminate (hopefully all) segfaults in pkgcachegen.cc and mmap.cc X-Git-Tag: 0.7.22~13^2 X-Git-Url: https://git.saurik.com/apt.git/commitdiff_plain/c5f44afc2446d738e30ea4c6021d4b60915546b1?ds=sidebyside eliminate (hopefully all) segfaults in pkgcachegen.cc and mmap.cc which can arise if cache doesn't fit into the mmap (Closes: #535218) This removes also the previously introduced SegfaultSignalHandler: The handler works, but is ugly by design... --- diff --git a/apt-pkg/contrib/mmap.cc b/apt-pkg/contrib/mmap.cc index 5f56178f4..ba4482131 100644 --- a/apt-pkg/contrib/mmap.cc +++ b/apt-pkg/contrib/mmap.cc @@ -19,7 +19,6 @@ #define _BSD_SOURCE #include #include -#include #include @@ -28,7 +27,6 @@ #include #include #include -#include #include /*}}}*/ @@ -139,24 +137,11 @@ bool MMap::Sync(unsigned long Start,unsigned long Stop) } /*}}}*/ -// DynamicMMapSegfaultHandler /*{{{*/ -// --------------------------------------------------------------------- -/* In theory, the mmap should never segfault because we check the available - size of our mmap before we use it, but there are a few reports out there - which state that the mmap segfaults without further notice. So this handler - will take care of all these segfaults which should never happen... */ -void DynamicMMapSegfaultHandler(int) -{ - _error->Error(_("Dynamic MMap segfaults, most likely because it ran out of room. " - "Please increase the size of APT::Cache-Limit. (man 5 apt.conf)")); - _error->DumpErrors(); - exit(EXIT_FAILURE); -} /*}}}*/ // DynamicMMap::DynamicMMap - Constructor /*{{{*/ // --------------------------------------------------------------------- /* */ -DynamicMMap::DynamicMMap(FileFd &F,unsigned long Flags,unsigned long WorkSpace) : +DynamicMMap::DynamicMMap(FileFd &F,unsigned long Flags,unsigned long WorkSpace) : MMap(F,Flags | NoImmMap), Fd(&F), WorkSpace(WorkSpace) { if (_error->PendingError() == true) @@ -187,13 +172,6 @@ DynamicMMap::DynamicMMap(unsigned long Flags,unsigned long WorkSpace) : if (_error->PendingError() == true) return; - if (_config->FindB("MMap::SegfaultHandler",true) == true) - { - struct sigaction sa; - sa.sa_handler = DynamicMMapSegfaultHandler; - sigaction(SIGSEGV, &sa, NULL); - } - #ifdef _POSIX_MAPPED_FILES // use anonymous mmap() to get the memory Base = (unsigned char*) mmap(0, WorkSpace, PROT_READ|PROT_WRITE, @@ -237,9 +215,9 @@ unsigned long DynamicMMap::RawAllocate(unsigned long Size,unsigned long Aln) unsigned long Result = iSize; if (Aln != 0) Result += Aln - (iSize%Aln); - + iSize = Result + Size; - + // try to grow the buffer while(Result + Size > WorkSpace) { @@ -258,7 +236,7 @@ unsigned long DynamicMMap::RawAllocate(unsigned long Size,unsigned long Aln) /* This allocates an Item of size ItemSize so that it is aligned to its size in the file. */ unsigned long DynamicMMap::Allocate(unsigned long ItemSize) -{ +{ // Look for a matching pool entry Pool *I; Pool *Empty = 0; @@ -283,17 +261,24 @@ unsigned long DynamicMMap::Allocate(unsigned long ItemSize) I->ItemSize = ItemSize; I->Count = 0; } - + + unsigned long Result = 0; // Out of space, allocate some more if (I->Count == 0) { - I->Count = 20*1024/ItemSize; - I->Start = RawAllocate(I->Count*ItemSize,ItemSize); - } + const unsigned long size = 20*1024; + I->Count = size/ItemSize; + Result = RawAllocate(size,ItemSize); + // Does the allocation failed ? + if (Result == 0 && _error->PendingError()) + return 0; + I->Start = Result; + } + else + Result = I->Start; I->Count--; - unsigned long Result = I->Start; - I->Start += ItemSize; + I->Start += ItemSize; return Result/ItemSize; } /*}}}*/ @@ -303,21 +288,14 @@ unsigned long DynamicMMap::Allocate(unsigned long ItemSize) unsigned long DynamicMMap::WriteString(const char *String, unsigned long Len) { - unsigned long Result = iSize; - // try to grow the buffer - while(Result + Len > WorkSpace) - { - if(!Grow()) - { - _error->Error(_("Dynamic MMap ran out of room. Please increase the size " - "of APT::Cache-Limit. Current value: %lu. (man 5 apt.conf)"), WorkSpace); - return 0; - } - } - if (Len == (unsigned long)-1) Len = strlen(String); - iSize += Len + 1; + + unsigned long Result = RawAllocate(Len+1,0); + + if (Result == 0 && _error->PendingError()) + return 0; + memcpy((char *)Base + Result,String,Len); ((char *)Base)[Result + Len] = 0; return Result; diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc index 7833d6476..e2934d1fa 100644 --- a/apt-pkg/pkgcachegen.cc +++ b/apt-pkg/pkgcachegen.cc @@ -53,14 +53,16 @@ pkgCacheGenerator::pkgCacheGenerator(DynamicMMap *pMap,OpProgress *Prog) : { // Setup the map interface.. Cache.HeaderP = (pkgCache::Header *)Map.Data(); - Map.RawAllocate(sizeof(pkgCache::Header)); + if (Map.RawAllocate(sizeof(pkgCache::Header)) == 0 && _error->PendingError() == true) + return; + Map.UsePools(*Cache.HeaderP->Pools,sizeof(Cache.HeaderP->Pools)/sizeof(Cache.HeaderP->Pools[0])); - + // Starting header *Cache.HeaderP = pkgCache::Header(); Cache.HeaderP->VerSysName = Map.WriteString(_system->VS->Label); Cache.HeaderP->Architecture = Map.WriteString(_config->Find("APT::Architecture")); - Cache.ReMap(); + Cache.ReMap(); } else { @@ -135,7 +137,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, pkgCache::VerIterator Ver = Pkg.VersionList(); map_ptrloc *LastVer = &Pkg->VersionList; - for (; Ver.end() == false; LastVer = &Ver->NextVer, Ver++) + for (; Ver != 0 && Ver.end() == false; LastVer = &Ver->NextVer, Ver++) { pkgCache::DescIterator Desc = Ver.DescriptionList(); map_ptrloc *LastDesc = &Ver->DescriptionList; @@ -143,7 +145,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, // don't add a new description if we have one for the given // md5 && language - for ( ; Desc.end() == false; Desc++) + for ( ; Desc != 0 && Desc.end() == false; Desc++) if (MD5SumValue(Desc.md5()) == CurMd5 && Desc.LanguageCode() == List.DescriptionLanguage()) duplicate=true; @@ -151,7 +153,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, continue; for (Desc = Ver.DescriptionList(); - Desc.end() == false; + Desc != 0 && Desc.end() == false; LastDesc = &Desc->NextDesc, Desc++) { if (MD5SumValue(Desc.md5()) == CurMd5) @@ -160,7 +162,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, *LastDesc = NewDescription(Desc, List.DescriptionLanguage(), CurMd5, *LastDesc); Desc->ParentPkg = Pkg.Index(); - if (NewFileDesc(Desc,List) == false) + if ((*LastDesc == 0 && _error->PendingError()) || NewFileDesc(Desc,List) == false) return _error->Error(_("Error occurred while processing %s (NewFileDesc1)"),PackageName.c_str()); break; } @@ -173,7 +175,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, pkgCache::VerIterator Ver = Pkg.VersionList(); map_ptrloc *LastVer = &Pkg->VersionList; int Res = 1; - for (; Ver.end() == false; LastVer = &Ver->NextVer, Ver++) + for (; Ver != 0 && Ver.end() == false; LastVer = &Ver->NextVer, Ver++) { Res = Cache.VS->CmpVersion(Version,Ver.VerStr()); if (Res >= 0) @@ -207,7 +209,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, // Skip to the end of the same version set. if (Res == 0) { - for (; Ver.end() == false; LastVer = &Ver->NextVer, Ver++) + for (; Ver != 0 && Ver.end() == false; LastVer = &Ver->NextVer, Ver++) { Res = Cache.VS->CmpVersion(Version,Ver.VerStr()); if (Res != 0) @@ -220,7 +222,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List, Ver->ParentPkg = Pkg.Index(); Ver->Hash = Hash; - if (List.NewVersion(Ver) == false) + if ((*LastVer == 0 && _error->PendingError()) || List.NewVersion(Ver) == false) return _error->Error(_("Error occurred while processing %s (NewVersion1)"), PackageName.c_str()); @@ -246,13 +248,13 @@ bool pkgCacheGenerator::MergeList(ListParser &List, map_ptrloc *LastDesc = &Ver->DescriptionList; // Skip to the end of description set - for (; Desc.end() == false; LastDesc = &Desc->NextDesc, Desc++); + for (; Desc != 0 && Desc.end() == false; LastDesc = &Desc->NextDesc, Desc++); // Add new description *LastDesc = NewDescription(Desc, List.DescriptionLanguage(), List.Description_md5(), *LastDesc); Desc->ParentPkg = Pkg.Index(); - if (NewFileDesc(Desc,List) == false) + if ((*LastDesc == 0 && _error->PendingError()) || NewFileDesc(Desc,List) == false) return _error->Error(_("Error occurred while processing %s (NewFileDesc2)"),PackageName.c_str()); } @@ -304,7 +306,7 @@ bool pkgCacheGenerator::MergeFileProvides(ListParser &List) unsigned long Hash = List.VersionHash(); pkgCache::VerIterator Ver = Pkg.VersionList(); - for (; Ver.end() == false; Ver++) + for (; Ver != 0 && Ver.end() == false; Ver++) { if (Ver->Hash == Hash && Version.c_str() == Ver.VerStr()) { @@ -370,7 +372,7 @@ bool pkgCacheGenerator::NewFileVer(pkgCache::VerIterator &Ver, // Link it to the end of the list map_ptrloc *Last = &Ver->FileList; - for (pkgCache::VerFileIterator V = Ver.FileList(); V.end() == false; V++) + for (pkgCache::VerFileIterator V = Ver.FileList(); V != 0 && V.end() == false; V++) Last = &V->NextFile; VF->NextFile = *Last; *Last = VF.Index(); @@ -419,14 +421,14 @@ bool pkgCacheGenerator::NewFileDesc(pkgCache::DescIterator &Desc, // Get a structure unsigned long DescFile = Map.Allocate(sizeof(pkgCache::DescFile)); if (DescFile == 0) - return 0; + return false; pkgCache::DescFileIterator DF(Cache,Cache.DescFileP + DescFile); DF->File = CurrentFile - Cache.PkgFileP; // Link it to the end of the list map_ptrloc *Last = &Desc->FileList; - for (pkgCache::DescFileIterator D = Desc.FileList(); D.end() == false; D++) + for (pkgCache::DescFileIterator D = Desc.FileList(); D != 0 && D.end() == false; D++) Last = &D->NextFile; DF->NextFile = *Last; @@ -460,6 +462,8 @@ map_ptrloc pkgCacheGenerator::NewDescription(pkgCache::DescIterator &Desc, Desc->ID = Cache.HeaderP->DescriptionCount++; Desc->language_code = Map.WriteString(Lang); Desc->md5sum = Map.WriteString(md5sum.Value()); + if (Desc->language_code == 0 || Desc->md5sum == 0) + return 0; return Description; } @@ -514,7 +518,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator Ver, if (OldDepVer != Ver) { OldDepLast = &Ver->DependsList; - for (pkgCache::DepIterator D = Ver.DependsList(); D.end() == false; D++) + for (pkgCache::DepIterator D = Ver.DependsList(); D != 0 && D.end() == false; D++) OldDepLast = &D->NextDepends; OldDepVer = Ver; } @@ -809,7 +813,7 @@ bool pkgMakeStatusCache(pkgSourceList &List,OpProgress &Progress, unsigned long EndOfSource = Files.size(); if (_system->AddStatusFiles(Files) == false) return false; - + // Decide if we can write to the files.. string CacheFile = _config->FindFile("Dir::Cache::pkgcache"); string SrcCacheFile = _config->FindFile("Dir::Cache::srcpkgcache"); @@ -861,8 +865,9 @@ bool pkgMakeStatusCache(pkgSourceList &List,OpProgress &Progress, { // Preload the map with the source cache FileFd SCacheF(SrcCacheFile,FileFd::ReadOnly); - if (SCacheF.Read((unsigned char *)Map->Data() + Map->RawAllocate(SCacheF.Size()), - SCacheF.Size()) == false) + unsigned long alloc = Map->RawAllocate(SCacheF.Size()); + if (alloc == 0 || SCacheF.Read((unsigned char *)Map->Data() + alloc, + SCacheF.Size()) == false) return false; TotalSize = ComputeSize(Files.begin()+EndOfSource,Files.end()); diff --git a/debian/changelog b/debian/changelog index e4da484d4..ea7e0469b 100644 --- a/debian/changelog +++ b/debian/changelog @@ -37,8 +37,8 @@ apt (0.7.22) UNRELEASED; urgency=low * versions with a pin of -1 shouldn't be a candidate (Closes: #355237) * prefer mmap as memory allocator in MMap instead of a static char array which can (at least in theory) grow dynamic - * add a segfault handler to MMap to show the Cache-Limit message, which - can be deactivated with MMap::SegfaultHandler=false (Closes: 535218) + * eliminate (hopefully all) segfaults in pkgcachegen.cc and mmap.cc + which can arise if cache doesn't fit into the mmap (Closes: #535218) * display warnings instead of errors if the parts dirs doesn't exist [ Michael Vogt ]