Skip to content

Commit

Permalink
Fix leak of dpkg cache when dpkginfo_init is called multiple times
Browse files Browse the repository at this point in the history
In the dpkginfo probe, the dpkg cache was allocated in dpkginfo_init.
However, dpkginfo_init can be called multiple times, leading the
cgCache pointer to be overridden and leaking the dpkg cache.
There are usually two dpkginfo probes running at the same time:
CPE and OVAL.

The opencache function is conditioned by the _init_done flag,
however, this flag was never set, so opencache was always called.

This change sets _init_done to 1 after opencache has been called,
and allocate the dpkg cache only when _init_done is 0.

Concurrent access to the cgCache static variable is now protected
by a statically initialized mutex in dpkginfo_probe.c.

We chose to keep cgCache as a static variable, because allocating
cgCache for every thread has big performance impact.
Also, calling new/delete revealed memory leaks in libapt.
Consequently, cgCache is initialized once and never freed.

The call to the cgCache->ReadOnlyOpen method is now done in the
dpkginfo_get_by_name function, so the dpkg cache is always up to date.
The cgCache->Close method is still called in dpkginfo_ini.

This change also removes the unused dpkg_mmap static variable
in dpkginfo-helper.cxx.
  • Loading branch information
0intro committed Sep 4, 2023
1 parent ed91feb commit f41f0ee
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 15 deletions.
20 changes: 8 additions & 12 deletions src/OVAL/probes/unix/linux/dpkginfo-helper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ using namespace std;
static int _init_done = 0;
static pkgCacheFile *cgCache = NULL;

static MMap *dpkg_mmap = NULL;

static int opencache (void) {
if (pkgInitConfig (*_config) == false) return 0;

Expand All @@ -37,8 +35,6 @@ static int opencache (void) {

if (pkgInitSystem (*_config, _system) == false) return 0;

if (!cgCache->ReadOnlyOpen(NULL)) return 0;

if (_error->PendingError () == true) {
_error->DumpErrors ();
return 0;
Expand All @@ -53,6 +49,8 @@ struct dpkginfo_reply_t * dpkginfo_get_by_name(const char *name, int *err)
pkgRecords Recs (cache);
struct dpkginfo_reply_t *reply = NULL;

if (!cgCache->ReadOnlyOpen(NULL)) return 0;

// Locate the package
pkgCache::PkgIterator Pkg = cache.FindPkg(name);
if (Pkg.end() == true) {
Expand Down Expand Up @@ -127,11 +125,15 @@ void dpkginfo_free_reply(struct dpkginfo_reply_t *reply)

int dpkginfo_init()
{
cgCache = new pkgCacheFile;
if (_init_done == 0)
if (_init_done == 0) {
cgCache = new pkgCacheFile;
if (opencache() != 1) {
delete cgCache;
cgCache = NULL;
return -1;
}
_init_done = 1;
}

return 0;
}
Expand All @@ -142,12 +144,6 @@ int dpkginfo_fini()
cgCache->Close();
}

delete cgCache;
cgCache = NULL;

delete dpkg_mmap;
dpkg_mmap = NULL;

return 0;
}

8 changes: 5 additions & 3 deletions src/OVAL/probes/unix/linux/dpkginfo_probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ struct dpkginfo_global {
};

static struct dpkginfo_global g_dpkg = {
.mutex = PTHREAD_MUTEX_INITIALIZER,
.init_done = -1,
};

Expand All @@ -78,9 +79,9 @@ int dpkginfo_probe_offline_mode_supported(void) {

void *dpkginfo_probe_init(void)
{
pthread_mutex_init (&(g_dpkg.mutex), NULL);

pthread_mutex_lock (&(g_dpkg.mutex));
g_dpkg.init_done = dpkginfo_init();
pthread_mutex_unlock (&(g_dpkg.mutex));
if (g_dpkg.init_done < 0) {
dE("dpkginfo_init has failed.");
}
Expand All @@ -92,8 +93,9 @@ void dpkginfo_probe_fini (void *ptr)
{
struct dpkginfo_global *d = (struct dpkginfo_global *)ptr;

pthread_mutex_lock (&(d->mutex));
dpkginfo_fini();
pthread_mutex_destroy (&(d->mutex));
pthread_mutex_unlock (&(d->mutex));

return;
}
Expand Down

0 comments on commit f41f0ee

Please sign in to comment.