[devel] [PATCH apt 3/3] Fix UB in pointer arithmetic
Dmitry V. Levin
ldv на altlinux.org
Вт Дек 10 02:56:56 MSK 2019
Commit 6d5e6a689d07de8feef2cbecb24bc42d5994861b aka 0.5.15lorg2-alt70~9
among other changes introduced UB in pointer arithmetic by casting raw
pointers to specific types.
Fix this by introducing two helpers for rebasing pointers in a safe way.
Co-developed-by: Aleksei Nikiforov <darktemplar на altlinux.org>
Fixes: 6d5e6a68 ("apt-pkg/pkgcachegen.{cc,h} changes")
---
apt/apt-pkg/Makefile.am | 1 +
apt/apt-pkg/cacheiterators.h | 14 ++++++++------
apt/apt-pkg/contrib/mmap.cc | 8 ++++----
apt/apt-pkg/pkgcachegen.cc | 27 +++++++++++----------------
apt/apt-pkg/rebase_pointer.h | 25 +++++++++++++++++++++++++
apt/apt-pkg/rpm/rpmlistparser.cc | 3 ++-
6 files changed, 51 insertions(+), 27 deletions(-)
create mode 100644 apt/apt-pkg/rebase_pointer.h
diff --git a/apt/apt-pkg/Makefile.am b/apt/apt-pkg/Makefile.am
index 4c0d234..d038d01 100644
--- a/apt/apt-pkg/Makefile.am
+++ b/apt/apt-pkg/Makefile.am
@@ -94,6 +94,7 @@ libapt_pkg_la_SOURCES = \
pkgsystem.h \
policy.cc \
policy.h \
+ rebase_pointer.h \
repository.cc \
repository.h \
scopeexit.h \
diff --git a/apt/apt-pkg/cacheiterators.h b/apt/apt-pkg/cacheiterators.h
index 9dffeb3..3c60cb8 100644
--- a/apt/apt-pkg/cacheiterators.h
+++ b/apt/apt-pkg/cacheiterators.h
@@ -34,6 +34,8 @@
#pragma interface "apt-pkg/cacheiterators.h"
#endif
+#include <apt-pkg/rebase_pointer.h>
+
// Package Iterator
class pkgCache::PkgIterator
{
@@ -87,7 +89,7 @@ class pkgCache::PkgIterator
{
if (Owner == 0 || Pkg == 0)
return;
- Pkg += static_cast<Package const *>(newMap) - static_cast<Package const *>(oldMap);
+ RebasePointer(Pkg, oldMap, newMap);
}
// Constructors
@@ -149,7 +151,7 @@ class pkgCache::VerIterator
{
if (Owner == 0 || Ver == 0)
return;
- Ver += static_cast<Version const *>(newMap) - static_cast<Version const *>(oldMap);
+ RebasePointer(Ver, oldMap, newMap);
}
inline VerIterator() : Ver(0), Owner(0) {};
@@ -222,7 +224,7 @@ class pkgCache::DepIterator
{
if (Owner == 0 || Dep == 0)
return;
- Dep += static_cast<Dependency const *>(newMap) - static_cast<Dependency const *>(oldMap);
+ RebasePointer(Dep, oldMap, newMap);
}
inline DepIterator(pkgCache &Owner,Dependency *Trg,Version * = 0) :
@@ -281,7 +283,7 @@ class pkgCache::PrvIterator
{
if (Owner == 0 || Prv == 0)
return;
- Prv += static_cast<Provides const *>(newMap) - static_cast<Provides const *>(oldMap);
+ RebasePointer(Prv, oldMap, newMap);
}
inline PrvIterator() : Prv(0), Type(PrvVer), Owner(0) {};
@@ -344,7 +346,7 @@ class pkgCache::PkgFileIterator
{
if (Owner == 0 || File == 0)
return;
- File += static_cast<PackageFile const *>(newMap) - static_cast<PackageFile const *>(oldMap);
+ RebasePointer(File, oldMap, newMap);
}
// Constructors
@@ -385,7 +387,7 @@ class pkgCache::VerFileIterator
{
if (Owner == 0 || FileP == 0)
return;
- FileP += static_cast<VerFile const *>(newMap) - static_cast<VerFile const *>(oldMap);
+ RebasePointer(FileP, oldMap, newMap);
}
inline VerFileIterator() : Owner(0), FileP(0) {};
diff --git a/apt/apt-pkg/contrib/mmap.cc b/apt/apt-pkg/contrib/mmap.cc
index 2064fc4..779d7a6 100644
--- a/apt/apt-pkg/contrib/mmap.cc
+++ b/apt/apt-pkg/contrib/mmap.cc
@@ -30,6 +30,7 @@
#include <apt-pkg/configuration.h>
#include <apt-pkg/mmap.h>
#include <apt-pkg/error.h>
+#include <apt-pkg/rebase_pointer.h>
#include <apti18n.h>
@@ -285,13 +286,12 @@ std::experimental::optional<map_ptrloc> DynamicMMap::Allocate(unsigned long Item
I->Count = size/ItemSize;
Pool* oldPools = Pools;
auto idxResult = RawAllocate(I->Count*ItemSize,ItemSize);
- if (Pools != oldPools)
- I += Pools - oldPools;
// Does the allocation failed ?
if (!idxResult)
return idxResult;
+ RebasePointer(I, oldPools, Pools);
Result = *idxResult;
I->Start = Result;
}
@@ -356,7 +356,7 @@ bool DynamicMMap::Grow(unsigned long long size)
Fd->Write(&C,sizeof(C));
}
- unsigned long const poolOffset = Pools - ((Pool*) Base);
+ const void *old_base = Base;
if (Fd != 0)
{
@@ -393,7 +393,7 @@ bool DynamicMMap::Grow(unsigned long long size)
memset((char*)Base + WorkSpace, 0, newSize - WorkSpace);
}
- Pools = (Pool*) Base + poolOffset;
+ RebasePointer(Pools, old_base, Base);
WorkSpace = newSize;
return true;
diff --git a/apt/apt-pkg/pkgcachegen.cc b/apt/apt-pkg/pkgcachegen.cc
index 56716b5..7a5a20c 100644
--- a/apt/apt-pkg/pkgcachegen.cc
+++ b/apt/apt-pkg/pkgcachegen.cc
@@ -26,6 +26,7 @@
#include <apt-pkg/strutl.h>
#include <apt-pkg/sptr.h>
#include <apt-pkg/pkgsystem.h>
+#include <apt-pkg/rebase_pointer.h>
#include <apti18n.h>
@@ -116,11 +117,11 @@ void pkgCacheGenerator::ReMap(void const * const oldMap, void const * const newM
Cache.ReMap(false);
- CurrentFile += (pkgCache::PackageFile*) newMap - (pkgCache::PackageFile*) oldMap;
+ RebasePointer(CurrentFile, oldMap, newMap);
for (size_t i = 0; i < _count(UniqHash); ++i)
if (UniqHash[i] != 0)
- UniqHash[i] += (pkgCache::StringItem*) newMap - (pkgCache::StringItem*) oldMap;
+ RebasePointer(UniqHash[i], oldMap, newMap);
for (auto i = Dynamic<pkgCache::PkgIterator>::toReMap.begin();
i != Dynamic<pkgCache::PkgIterator>::toReMap.end(); ++i)
@@ -269,11 +270,8 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
continue;
}
- if (oldMap != Map.Data())
- {
- Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
- oldMap = Map.Data();
- }
+ RebasePointer(Last, oldMap, Map.Data());
+ oldMap = Map.Data();
// Skip to the end of the same version set.
if (Res == 0)
@@ -296,8 +294,7 @@ bool pkgCacheGenerator::MergeList(ListParser &List,
return _error->Error(_("Error occurred while processing %s (NewVersion%d)"),
PackageName.c_str(), 1);
- if (oldMap != Map.Data())
- Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
+ RebasePointer(Last, oldMap, Map.Data());
*Last = *verindex;
Ver->ParentPkg = Pkg.Index();
@@ -604,8 +601,9 @@ bool pkgCacheGenerator::ListParser::NewDepends(pkgCache::VerIterator &Ver,
for (pkgCache::DepIterator D = Ver.DependsList(); D.end() == false; D++)
OldDepLast = &D->NextDepends;
OldDepVer = Ver;
- } else if (oldMap != Owner->Map.Data())
- OldDepLast += (map_ptrloc*) Owner->Map.Data() - (map_ptrloc*) oldMap;
+ } else {
+ RebasePointer(OldDepLast, oldMap, Owner->Map.Data());
+ }
// Is it a file dependency?
if (PackageName[0] == '/')
@@ -745,11 +743,8 @@ std::experimental::optional<map_ptrloc> pkgCacheGenerator::WriteUniqString(const
if ((!Item) || (!idxString))
return std::experimental::optional<map_ptrloc>();
- if (oldMap != Map.Data())
- {
- Last += (map_ptrloc*) Map.Data() - (map_ptrloc*) oldMap;
- I += (pkgCache::StringItem*) Map.Data() - (pkgCache::StringItem*) oldMap;
- }
+ RebasePointer(Last, oldMap, Map.Data());
+ RebasePointer(I, oldMap, Map.Data());
*Last = *Item;
diff --git a/apt/apt-pkg/rebase_pointer.h b/apt/apt-pkg/rebase_pointer.h
new file mode 100644
index 0000000..2bbabea
--- /dev/null
+++ b/apt/apt-pkg/rebase_pointer.h
@@ -0,0 +1,25 @@
+#ifndef PKGLIB_REBASE_POINTER_H
+#define PKGLIB_REBASE_POINTER_H
+
+template <class T>
+static inline T*
+GetRebasedPointer(T*, const void *, const void *)
+__attribute__((__warn_unused_result__));
+
+template <class T>
+static inline T*
+GetRebasedPointer(T* ptr, const void *old_base, const void *new_base)
+{
+ // uintptr_t is a type with well-defined integer overflow semantics
+ uintptr_t diff = (uintptr_t) new_base - (uintptr_t) old_base;
+ return (T*) ((uintptr_t) ptr + diff);
+}
+
+template <class T>
+static inline void
+RebasePointer(T* &ptr, const void *old_base, const void *new_base)
+{
+ ptr = GetRebasedPointer(ptr, old_base, new_base);
+}
+
+#endif
diff --git a/apt/apt-pkg/rpm/rpmlistparser.cc b/apt/apt-pkg/rpm/rpmlistparser.cc
index 9b2e9ad..4aeb937 100644
--- a/apt/apt-pkg/rpm/rpmlistparser.cc
+++ b/apt/apt-pkg/rpm/rpmlistparser.cc
@@ -25,6 +25,7 @@
#include <apt-pkg/strutl.h>
#include <apt-pkg/crc-16.h>
#include <apt-pkg/tagfile.h>
+#include <apt-pkg/rebase_pointer.h>
#include <apti18n.h>
@@ -56,7 +57,7 @@ rpmListParser::rpmListParser(RPMHandler *Handler)
for (auto iter: *SeenPackages)
{
- tmp.insert(iter + (static_cast<const char *>(newMap) - static_cast<const char *>(oldMap)));
+ tmp.insert(GetRebasedPointer(iter, oldMap, newMap));
}
SeenPackages->swap(tmp);
--
ldv
Подробная информация о списке рассылки Devel