Remap StringView instances pointing into the cache

It turns out that StringViews might need to be remapped in some
places because they come from the cache. For example, some sites
pass a Ver.VerStr() to NewProvides().

Such a StringView would become invalid during the duration of
the call if the cache is remapped, causing the program to die
with a segmentation fault.

We can take care of those issues by remapping string views in
the same way we remap all the iterators. String views are only
remapped if they point into the cache though, this allows us
to write more generic code on the callee site without having
to check whether the view points into the cache or not.
That's not as efficient as possible, but the overhead does not
appear to be measurable.

Closes: #812251

diff --git a/apt-pkg/pkgcachegen.cc b/apt-pkg/pkgcachegen.cc
index a472babeb9f1e1210aac2b641a7bd895637235cf..d8f161e6de44b1998a345db6a85f65e8ff3f0007 100644 (file)
--- a/apt-pkg/pkgcachegen.cc
+++ b/apt-pkg/pkgcachegen.cc
@@ -177,6 +177,14 @@ void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newM
    for (std::vector<pkgCache::RlsFileIterator*>::const_iterator i = Dynamic<pkgCache::RlsFileIterator>::toReMap.begin();
        i != Dynamic<pkgCache::RlsFileIterator>::toReMap.end(); ++i)
       (*i)->ReMap(oldMap, newMap);
    for (std::vector<pkgCache::RlsFileIterator*>::const_iterator i = Dynamic<pkgCache::RlsFileIterator>::toReMap.begin();
        i != Dynamic<pkgCache::RlsFileIterator>::toReMap.end(); ++i)
       (*i)->ReMap(oldMap, newMap);

+   for (APT::StringView* ViewP : Dynamic<APT::StringView>::toReMap) {
+      // Ignore views outside of the cache.
+      if (ViewP->data() < static_cast<const char*>(oldMap)
+        || ViewP->data() > static_cast<const char*>(oldMap) + oldSize)
+        continue;
+      const char *data = ViewP->data() + (static_cast<const char*>(newMap) - static_cast<const char*>(oldMap));
+      *ViewP = StringView(data , ViewP->size());
+   }

 }                                                                      /*}}}*/
 // CacheGenerator::WriteStringInMap                                    /*{{{*/
 map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String,
 }                                                                      /*}}}*/
 // CacheGenerator::WriteStringInMap                                    /*{{{*/
 map_stringitem_t pkgCacheGenerator::WriteStringInMap(const char *String,


@@ -503,6 +511,7 @@ bool pkgCacheGenerator::AddNewDescription(ListParser &List, pkgCache::VerIterato
 /* This creates a new group structure and adds it to the hash table */
 bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name)
 {
 /* This creates a new group structure and adds it to the hash table */
 bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name)
 {

+   Dynamic<StringView> DName(Name);

    Grp = Cache.FindGrp(Name);
    if (Grp.end() == false)
       return true;
    Grp = Cache.FindGrp(Name);
    if (Grp.end() == false)
       return true;


@@ -537,6 +546,8 @@ bool pkgCacheGenerator::NewGroup(pkgCache::GrpIterator &Grp, StringView Name)
 bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name,
                                        StringView Arch) {
    pkgCache::GrpIterator Grp;
 bool pkgCacheGenerator::NewPackage(pkgCache::PkgIterator &Pkg, StringView Name,
                                        StringView Arch) {
    pkgCache::GrpIterator Grp;

+   Dynamic<StringView> DName(Name);
+   Dynamic<StringView> DArch(Arch);

    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);
    if (unlikely(NewGroup(Grp, Name) == false))
       return false;
    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);
    if (unlikely(NewGroup(Grp, Name) == false))
       return false;


@@ -1014,6 +1025,9 @@ bool pkgCacheListParser::NewDepends(pkgCache::VerIterator &Ver,
 {
    pkgCache::GrpIterator Grp;
    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);
 {
    pkgCache::GrpIterator Grp;
    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);

+   Dynamic<StringView> DynPackageName(PackageName);
+   Dynamic<StringView> DynArch(Arch);
+   Dynamic<StringView> DynVersion(Version);

    if (unlikely(Owner->NewGroup(Grp, PackageName) == false))
       return false;
 
    if (unlikely(Owner->NewGroup(Grp, PackageName) == false))
       return false;
 


@@ -1081,6 +1095,9 @@ bool pkgCacheListParser::NewProvides(pkgCache::VerIterator &Ver,
                                                uint8_t const Flags)
 {
    pkgCache const &Cache = Owner->Cache;
                                                uint8_t const Flags)
 {
    pkgCache const &Cache = Owner->Cache;

+   Dynamic<StringView> DynPkgName(PkgName);
+   Dynamic<StringView> DynArch(PkgArch);
+   Dynamic<StringView> DynVersion(Version);

 
    // We do not add self referencing provides
    if (Ver.ParentPkg().Name() == PkgName && (PkgArch == Ver.ParentPkg().Arch() ||
 
    // We do not add self referencing provides
    if (Ver.ParentPkg().Name() == PkgName && (PkgArch == Ver.ParentPkg().Arch() ||


@@ -1134,6 +1151,8 @@ bool pkgCacheListParser::NewProvidesAllArch(pkgCache::VerIterator &Ver, StringVi
    pkgCache &Cache = Owner->Cache;
    pkgCache::GrpIterator Grp = Cache.FindGrp(Package);
    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);
    pkgCache &Cache = Owner->Cache;
    pkgCache::GrpIterator Grp = Cache.FindGrp(Package);
    Dynamic<pkgCache::GrpIterator> DynGrp(Grp);

+   Dynamic<StringView> DynPackage(Package);
+   Dynamic<StringView> DynVersion(Version);

 
    if (Grp.end() == true)
       return NewProvides(Ver, Package, Cache.NativeArch(), Version, Flags);
 
    if (Grp.end() == true)
       return NewProvides(Ver, Package, Cache.NativeArch(), Version, Flags);