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 5f56178f4230e1296e0b71ff4910b61b0b0de714..ba4482131d4c1fd7b5ddc4388d3b16c9b93ed0a1 100644 (file)
--- a/apt-pkg/contrib/mmap.cc
+++ b/apt-pkg/contrib/mmap.cc
@@ -19,7 +19,6 @@
 #define _BSD_SOURCE
 #include <apt-pkg/mmap.h>
 #include <apt-pkg/error.h>
 #define _BSD_SOURCE
 #include <apt-pkg/mmap.h>
 #include <apt-pkg/error.h>

-#include <apt-pkg/configuration.h>

 
 #include <apti18n.h>
 
 
 #include <apti18n.h>
 


@@ -28,7 +27,6 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>
 #include <unistd.h>
 #include <fcntl.h>
 #include <stdlib.h>

-#include <signal.h>

 
 #include <cstring>
                                                                        /*}}}*/
 
 #include <cstring>
                                                                        /*}}}*/


@@ -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 - 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)
              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 (_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,
 #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);
    unsigned long Result = iSize;
    if (Aln != 0)
       Result += Aln - (iSize%Aln);

-   
+

    iSize = Result + Size;
    iSize = Result + Size;

-   
+

    // try to grow the buffer
    while(Result + Size > WorkSpace)
    {
    // 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)
 /* 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;
    // 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;
    }
       I->ItemSize = ItemSize;
       I->Count = 0;
    }

-   
+
+   unsigned long Result = 0;

    // Out of space, allocate some more
    if (I->Count == 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--;
 
    I->Count--;

-   unsigned long Result = I->Start;
-   I->Start += ItemSize;  
+   I->Start += ItemSize;

    return Result/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 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);
    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;
    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 7833d6476645e905820d5d00c4a0ba3886df41f1..e2934d1faed97040419f0891947a70cbaf1e5f0f 100644 (file)
--- 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();
    {
       // 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]));
       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"));
       // 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
    {
    }
    else
    {


@@ -135,7 +137,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
         pkgCache::VerIterator Ver = Pkg.VersionList();
         map_ptrloc *LastVer = &Pkg->VersionList;
 
         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;
         {
            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
 
            // 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;
               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();
               continue;
            
            for (Desc = Ver.DescriptionList();

-                Desc.end() == false; 
+                Desc != 0 && Desc.end() == false;

                 LastDesc = &Desc->NextDesc, Desc++)
            {
               if (MD5SumValue(Desc.md5()) == CurMd5) 
                 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();
                  
                  *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;
               }
                     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;
       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)
       {
         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)
       {
       // 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)
         {
            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;
 
       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());
 
         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
       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();
 
 
       // 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());
    }
 
         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();
 
       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())
         {
       {
         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;
    
    // 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();
       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)
    // 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;
 
    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;
       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());
    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;
 }
 
    return Description;
 }


@@ -514,7 +518,7 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator Ver,
    if (OldDepVer != Ver)
    {
       OldDepLast = &Ver->DependsList;
    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;
    }
         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;
    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");
    // 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);
    {
       // 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());
         return false;
 
       TotalSize = ComputeSize(Files.begin()+EndOfSource,Files.end());

diff --git a/debian/changelog b/debian/changelog
index e4da484d46ad521f0d2631d86f1c0575524c0908..ea7e0469bdf514d16f24c1d1e05da992f729ce1a 100644 (file)
--- 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
   * 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 ]
   * display warnings instead of errors if the parts dirs doesn't exist
 
   [ Michael Vogt ]