[MIR] pcp package

Bug #1700827 reported by Eric Desrochers on 2017-06-27
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Fix Released
pcp (Debian)
Fix Released
pcp (Ubuntu)
Eric Desrochers

Bug Description

pcp | 3.11.10 | artful/universe | source, amd64, arm64, armhf, i386, ppc64el, s390x

This is a valuable source of metrics for large deployments. There are numerous users with large installations interested in having it in fully supported state.

National Vulnerability Database:

OSS security:
Ubuntu CVE tracker: none

All CVEs handled by upstream.

Executables which have the suid or sgid bit set: none
Executables in /sbin, /usr/sbin: none
Packages which install services / daemons (/etc/init.d/*, /etc/init/*, /lib/systemd/system/*)
pcp-manager: /etc/init.d/pmmgr
pcp: /etc/init.d/{pcp, pmcd, pmie, pmlogger, pmproxy}
    Above checks done with: http://paste.ubuntu.com/24916305/
Packages which open privileged ports (ports < 1024): none
Add-ons and plugins to security-sensitive software (filters, scanners, UI skins, etc): none
[Quality assurance]
Easy to make it work after install (no extra actions required)
Does not ask debconf questions.
No outstanding bugs affecting usability.

There are some open bugs:


There is a testsuite but it’s not designed to be run at build. It requires installed and operational PCP.

No debian/watch file - needs to be added.

No debian/test/ - need autopkgtest (dep8)

All dependencies met in main (including recommended)

libpapi-dev is a universe build-time dependency introduced with zesty.
[Standards compliance]
Lintian report https://<email address hidden>#pcp_3.11.10

I haven’t found any violations of FHS.

Owning team? Server team?

[Background information]
Package description:
System level performance monitoring and performance management
 Performance Co-Pilot (PCP) is a framework and services to
 support system-level performance monitoring and performance
 The Performance Co-Pilot provides a unifying abstraction for
 all of the interesting performance data in a system, and allows
 client applications to easily retrieve and process any subset of
 that data.

Eric Desrochers (slashd) on 2017-06-27
tags: added: sts
Changed in pcp (Ubuntu):
assignee: nobody → Dariusz Gadomski (dgadomski)
Eric Desrochers (slashd) on 2017-06-27
description: updated
Eric Desrochers (slashd) on 2017-06-27
description: updated
Eric Desrochers (slashd) wrote :

The following, must first be addressed before this package would be accepted into main :

- tracking down upstream maintenance
- fix segfaulting binaries
- evaluating all dependencies
- base dep8 tests included

Eric Desrochers (slashd) wrote :

Other discussions ....

# systemd or sysv ??

Do we need to port it to systemd or are sysv script is fine?

In general sysv is fine, but there are a few issues due using the "old" calls e.g. if you have PCP running in a container it will collide on the Host (no service start, failing package install) - things like that have to be addressed either in the sysV script or by creating a better service file.

# dependencies

Christian :
It seems dependencies were checked in Xenial only.
In latter releases e.g. Artful pcp has grown a dependency to libpapi5 which is in universe.

So the MIR as it is now would either:
- include adding and carrying a delta to whatever added libpapi (didn't check the source if that is doable)
- add another full MIR process for source "papi"

Dariusz Gadomski (dgadomski) wrote :

Reported main inclusion request for the dependency (papi): bug #1704130.

summary: - MIR -- pcp package
+ [MIR] pcp package
Eric Desrochers (slashd) wrote :

Dariusz has created some autopkgtest scripts doing invocation of some of the cli pcp tools, ...

After a few run from Dariusz and I, both test scripts successfully passed.

adt-run [09:10:04]: @@@@@@@@@@@@@@@@@@@@ summary
check_daemons.py PASS
check_cli_tools.py PASS

Next step is to submit the autopkgtest proposal to Debian upstream.

- Dariusz & Eric

Dariusz Gadomski (dgadomski) wrote :

I have reported a Debian bug with request to include the autopkgtests.


Changed in pcp:
status: Unknown → New
Changed in pcp (Debian):
status: Unknown → New
Seth Arnold (seth-arnold) wrote :
Download full text (4.2 KiB)

I reviewed pcp version 3.12.0build2 as checked into artful. This should
not be considered a full security audit but rather a quick gauge of

PCP has a long history of being the go-to-package for performance
monitoring, especially across a network.

There's five CVEs in our database; CVE-2012-3418 is multi-purpose and
addresses what feels like twenty or so individual issues. The issues
that it raises feels very similar to what I found while reviewing pcp.

I should stress that the threat model of PCP may mean that nothing
I've found is actually a security issue -- however, the coding style
is what might have been appropriate twenty years ago but is no longer
appropriate today.

Much of what I read was using C as shell scripts. This is marginally
suitable in the case of PMDAs calling programs to collect statistics
but is far from ideal for the more structural components.

None of the snprintf() calls checked for error returns, even when the
inputs come from outside the process. Overflows could cause surprising
results as truncated commands are executed.

There are at least half a dozen functions that serve as system()
or popen() replacements or wrappers. I also saw several functions that
serve as execve() wrappers; I didn't annotate those, since they are less
likely to be a source of problems. Since most of the strings are built
from pieces anyway the much safer execve() style of execution should be
used instead. (system() and popen() are really best suited strictly for
instances of users supplying exact commands they want run.)

There are several cases of unchecked sprintf() calls that could overflow
buffers if provided with unsafe input. Again, I'd like to stress that
these may not be security flaws in the threat model of PCP but their
presence is worrying.

cppcheck reports roughly 117 issues. Some may be false positives and
some may have very limited impact (e.g. 78 instances of realloc()
mis-use that leaks memory on memory allocation failure) but this is an
easy source of objectively useful things to fix.

The documentation is superb. I was excited reading about PCP and wanted
to use it myself. I really do hope the PCP team can make the big changes
I feel are necessary for today's environments.

Security team NAK for promoting pcp to main.

Here's the notes I collected in the hopes that they are useful for

command executors
- start_cmd from src/pmdas/logger/util.c
- start_cmd from src/pmdas/pipe/util.c
- pmmgr_daemon::poll
- pmmgr_configurable::wrap_system
- do_shell
- local_pipe

collectl2pcp unsafe filename input mechanism to popen()

watch() from ./src/dbpmda/src/util.c unsafe filename use

pcpScript() unsafe system() via name and action; unchecked snprintf() call

lex() from ./src/libpcp/src/pmns.c unsafe bin_dir, fname use

popen_uncompress() from ./src/libpcp/src/logutil.c unsafe cmd, fname, pipecmd use

pmflush() from ./src/libpcp/src/util.c unsafe fname use

OpenViewDialog::openView() from ./src/pmchart/view.cpp unsafe PCP_VAR_DIR
and path use

OpenViewDialog::openView() _fname buffer overflow if path parameter is >

OpenViewDialog::openView() cmd buffer overflow if _fname is > MAXPATHLEN



Frank Ch. Eigler (fche) wrote :

Hi, Seth -

On Thu, Aug 17, 2017 at 07:15:01AM -0000, Seth Arnold wrote:
> I reviewed pcp version 3.12.0build2 as checked into artful. This should
> not be considered a full security audit but rather a quick gauge of
> maintainability.


> [...]
> None of the snprintf() calls checked for error returns, even when the
> inputs come from outside the process. Overflows could cause surprising
> results as truncated commands are executed.

These (and sprintf) are generally used with data coming from trusted
sources such as kernel /proc files. Their format is relatively fixed,
and is generally not amenable to end-user interference, so large
buffers and unchecked snprintf() seem risk-free. I suppose an
assert() wouldn't hurt, just be unsightly.

> There are at least half a dozen functions that serve as system()
> or popen() replacements or wrappers. [...]

Other than their existence, what did you see wrong?

> [...]
> Security team NAK for promoting pcp to main.
> Here's the notes I collected in the hopes that they are useful for
> someone: [...]

Thanks! It would be very useful to see individual bug reports about
that subset of issues that you consider to earn that NAK. Then we can
focus on analyzing them in detail - to see if there is any real
security risk; to discuss why the code is the way it is; to track
fixes of actual problems. Your rough list of notes is good but not
easily actionable.

- FChE

Ken McDonell (kenj) wrote :

Apropos the CVE's all of these have been completely addressed in past PCP releases.
CVE-2012-5530 - fixed in PCP 3.6.10 (released 19 Nov 2012)
CVE-2012-3421, CVE-2012-3420, CVE-2012-3419 and CVE-2012-3418 - all fixed in PCP 3.6.5 (released 16 Aug 2012)
CVE-2001-0823 - fixed in PCP 2.2.1 (released 21 Jun 2001)

I am in agreement with Frank, that we'd really like to see some specific issues identified that are barriers to inclusion of PCP in main.

If we had this list we're more than happy to work through them ... based on 20+ years of experience with PCP development and deployment in many big and ugly environments, I am confident that most of them will turn out to be false positives, others can be fixed.

Thanks for the effort so far with the review.

Changed in pcp:
status: New → Fix Released
Changed in pcp (Debian):
status: New → Fix Released
Seth Arnold (seth-arnold) wrote :

Hi Frank, Ken;

I'd be happy to expand my notes for an email or file bug reports for specific items if you'd feel it would be more useful than generic platitudes like "check all snprintf() calls for error returns", "check sprintf() string inputs for proper lengths", "fix most of the cppcheck results", "switch most executions away from system() towards execve()", "switch malloc(a*b) to calloc(a,b)" etc.

I made note of the system() and popen() wrappers as I found them with the intention of re-grepping the sources for the new functions and looking for further instances of potentially unsafe inputs being handed to sh. I never returned to this, but did want to point out that these wrappers are encouraging string-based "command line" execution rather than using the safer execve()-based methods (even when the execve() method would have otherwise been more convenient).


Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in pcp (Ubuntu):
status: New → Confirmed
Eric Desrochers (slashd) wrote :

@Dariusz, Seth, Ken, Frank and anyone else involved in this discussion.

What would you guys think if we schedule a hangout call between Canonical & PCP upstream folks to discuss about all this ?


Ken McDonell (kenj) wrote :

On 29/08/17 10:44, Eric Desrochers wrote:
> @Dariusz, Seth, Ken, Frank and anyone else involved in this discussion.
> What would you guys think if we schedule a hangout call between
> Canonical & PCP upstream folks to discuss about all this ?

Sounds good to me ... we may be a little timezone challenged ... I'm in
Melbourne, Frank's in Toronto, ...

Eric Desrochers (slashd) wrote :

Hi Ken/Frank,

Timezone is always challenging ;)

Can you guys provide some availabilities (2-3) for let's say next week ? and I'll see what I can do to respect everyone's agenda.

If the timezone is too much of a challenge, do you mind if we do the meeting with Frank ? and then we can give provide you a summary of the discussion and maybe continue the conversation with an email thread ?

Feel free to continue the conversation via email, my canonical email address : <email address hidden>


Nathan Scott (nathans) wrote :

Hi all,

I was only just made aware of this bug today, apologies for not being
involved earlier.

I'm involved with PCP upstream and Debian packaging, and I'm in the
same $TZ as Ken - I'd be quite interested in chatting about the PCP
packages in Ubuntu.

| "I'd be happy to expand my notes for an email"

Yes please; let's leave mass opening of individual bugs until after an
initial pass, as that'll generate alot of email noise on the PCP list.


Ken McDonell (kenj) wrote :


We've written replacements for system() and popen() that use execvp() and do not call /bin/sh -c "some command". These are __pmProcessExec() and __pmProcessPipe() in libpcp.

All uses of system() and popen() in the core libpcp library have been converted to the new routines.

There are 16 other uses of popen() in C code outside the core libpcp library that will be converted shortly.
There is one popen() in some Python code that will need additional thought/review.

There are only a handfull of other system() uses that do not involve literal strings ... we'll work through those.

And I've made some changes for the x = realloc(x, ...) pattern, but so far all of these are on fatal error paths that lead directly to exit(), so I'll continue to work on these as time permits, but don't view these as a significant risk.

Nathan Scott (nathans) wrote :

Update #2.

All unchecked calls to the [v]s[n]printf family of string manipulation
routines have been fixed in the github pcp master branch now as well.

That area, and the __pmProcess* APIs Ken has added (and associated, on
going conversion of system/popen/... call sites) covers most, if not
all, of Seths original list I think.

We have an upstream PCP release (pcp-3.12.2) planned for Oct 18th. If
another review could be done within the next week or two, that would
give us the remaining time to tackle any outstanding issues found by
2nd review & it might serve as a good base version for Ubuntu's need.


Seth Arnold (seth-arnold) wrote :

Hello Frank, Ken, and Nathan,

Very nice work.

I'm impressed with the changes I see in the PCP repository. The changes to address my feedback are thorough and thoughtful, and unrelated changes were of universal high-quality. (I especially enjoyed the hanoi and chains test loads!)

Eric, security team ACK for promoting pcp version 3.12.2 or any newer version to main in Ubuntu.

Many thanks to everyone.

Changed in pcp (Ubuntu):
assignee: Dariusz Gadomski (dgadomski) → nobody
Eric Desrochers (slashd) on 2017-11-08
Changed in pcp (Ubuntu):
status: Confirmed → In Progress
Eric Desrochers (slashd) on 2017-11-08
Changed in pcp (Ubuntu):
importance: Undecided → Medium
Dariusz Gadomski (dgadomski) wrote :

I believe it's really useful to also include papi [1] in main as it is a dependency of pcp and offers support for the performance counter hardware in many processor types providing a low-level source of metrics.
This ofc seems like an important feature in a performance monitoring suite.

[1] http://icl.utk.edu/papi/

I'm doing this review (I'm not done though).

As a preliminary comment as to my view of the state of the package (as notes to self so I don't forget):

- Why is this set as a native package? It's rather obviously not one, as it also ships and works on other distributions.
- pcp is still using compat 5, and old-format rules file. I think given its complexity it would really benefit (from a maintainability point-of-view) if the packaging was modernized -- compat level 9, using simpler dh-style rules, etc.

Changed in pcp (Ubuntu):
assignee: nobody → Mathieu Trudel-Lapierre (cyphermox)
Eric Desrochers (slashd) wrote :

> Why is this set as a native package? It's rather obviously not one, as it also ships and works on other distributions.

I don't know why, maybe Nathan Scott will have more context, but yeah this should be set to non-native as it is clearly also useful outside Debian context.


* When to use a native vs a non-native package
Default to making packages non-native. You should only use a native Debian package when it is clear that the package would not be useful outside the context of a Debian system, and would never be distributed except packaged for Debian or its derivatives. Even if the software is currently only available in Debian, if someone could reasonably use the software on another distribution or on another operating system, then the package should be non-native. A few examples of normal (non-native) packages are: libc6, apache, phpmyadmin. But lintian, dpkg and some other tools are purely developed for Debian, and make no sense being released in another distribution; these belong in native Debian packages.

Nathan Scott (nathans) wrote :

> - Why is this set as a native package?

This is for convenience of PCP developers - in particular,
both Ken and I regularly build and use the .deb packages in
our testing processes. The PCP project also release binary
debs with each release using the local (native) packaging -

FWLIW, it has been this way (in Debian) for over ten years.
It would be quite disruptive to our processes to rework it,
I suspect, but that's not out of the question (if there was
some way to replace our existing upstream package builds).

> pcp is still using compat 5

I believe Eric has been working on these aspects, and we've
exchanged a few mails - there's still some way to go though
to bring things completely up to date, AIUI.


Using compat 5 isn't wrong per se, but I did notice the need to modernize the packaging, and discussed this with Eric already. Using newer packaging makes it easier to maintain the package in the long run, as new developers may not know of or understand older tricks. A promise to keep improving the setup is enough for me. :)

As for using native packaging, that seems to me like an invitation to patch the package directly, without a patch system, when there are issues. Using a proper 3.0 (quilt) package would make it easier to separate a formal release's code with any further patches applied (especially when it comes to cherry-picking patches from upstream for upload to stable releases). This will make it much easier for Ubuntu (or Debian) developers to help with the pcp upstream project by sending their patches in a much more obvious way -- no need to dig in the code to figure out what the delta is. Furthermore, any change to the packaging would then need to be done by way of another upstream version -- you'd likely get version numbers that don't match what was officially released upstream for all distros (for example, you could release 5.5.2, but Debian/Ubuntu might ship if some extra changes had to be applied to the packaging).

I'm tempted to block inclusion in main on that reason, but I'll confer with my colleagues on the MIR team.


Please make sure there is a team subscriber for pcp bugs, this is a requirement for inclusion in main. This may be the Server Team, but they should know about it and agree to sign up for the extra work (however small it may be).

Changed in pcp (Ubuntu):
assignee: Mathieu Trudel-Lapierre (cyphermox) → Eric Desrochers (slashd)
Matthias Klose (doko) wrote :

+1 on converting the package to a 3.0 quilt format, plus using debhelper 9 and fixing the plethora of lintian warnings.

Bonus points to accept the hardening flags in the build, using parallel builds, and fixing the Multi-Arch hints.

Nathan Scott (nathans) wrote :

| Using a proper 3.0 (quilt) package would make it easier to [...]

This sounds like a good way to go. I'll assume Eric (or someone else from
Canonical/Ubuntu) is going to follow up with us on this ... chatting to Ken
earlier, this is a bit beyond our knowledge and it'll be best all round if
the experts are driving this. We'll help out with any makefile/build fixes
where we can to assist with integrating the updated packaging.


Eric Desrochers (slashd) wrote :

Hi Natha,

I'll take care of the dpkg-source format change from "3.0 (native)" to "3.0 (quilt)".

- Eric

Eric Desrochers (slashd) wrote :


Looking at the update_excuses page[1], I noticed pcp (3.12.2) is stuck in bionic-proposed[2] for 11 days now and was set as "Not considered"

At first glance, it seems the pkg failed to build because of a missing Build-Depends "libqt5svg5-dev"

[1] update excuses page

[2] - rmadison
 pcp | 3.12.2 | bionic-proposed/universe | source, ppc64el

- Eric

Eric Desrochers (slashd) wrote :

Debian bugs related to my above comment ^

# Missing dependency

# Convert from 3.0 (native) to 3.0 (quilt)

- Eric

Eric Desrochers (slashd) wrote :

I have reported another debian bug for pmatop (as discovered by Christian)


Will have a look when I have more available time.

Eric Desrochers (slashd) wrote :

# Missing build dependency for Ubuntu :
https://bugs.launchpad.net/bugs/1733619 (related to debbug #881649)

Eric Desrochers (slashd) wrote :

# pcp - convert dpkg-source format to 3.0 (quilt)

Eric Desrochers (slashd) wrote :

Nathan have committed upstream the two changes I have submitted :

* [894c007] build: switch deb builds to using the quilt-based format
 - LP: #1733646
 - debbug: #881650

* [e165a12] build: add in a missing a Qt5-specific svg library dependency
 - LP: #1733619
 - Debbug: #881649

Once the 2 above changes in debian unstable, I'll proceed with the upload into bionic to unblock the package build which is currently failing due to the missing dependency in both Debian[1] and Ubuntu[2].

[1] - https://buildd.debian.org/status/logs.php?pkg=pcp
[2] - https://launchpad.net/ubuntu/+source/pcp

Thanks Nathan

- Eric

Ken McDonell (kenj) wrote :

With the upstream changes mentioned above and a couple of tweaks (everything up to and including commit f09cac4), I've been able to complete a build, package, install and start PCP operation on all the machines in my PCP QA Farm.

This means:
amd64 FreeBSD 10.2-RELEASE, amd64 OpenBSD 5.8
i386 Darwin 10.8.0, FreeBSD 9.3-RELEASE-p30, NetBSD 6.1.5
i686 Debian 7.11, Debian 9.1, Fedora 25, Fedora 26, LinuxMint 15,
 openSUSE 13.2, Ubuntu 17.04
i86pc OpenIndiana Development oi_151.1.8
x86_64 Arch Linux, CentOS6.9, CentOS Linux7.3.1611, CentOS Linux7.3.1611,
 CentOS Linux7.3.1611, Debian 8.9, Debian 9.1, Fedora 21,
 Fedora 23, Fedora 25, Fedora 26, Gentoo 2.2, LinuxMint 17.3,
 MandrivaLinux 2011.0, openSUSE 42.2, openSUSE 42.3, RHEL Server
 6.9, RHEL Server 7.3, Slackware "14.2", SUSE SLES12 SP0, Ubuntu
 14.04, Ubuntu 15.04, Ubuntu 16.04, Ubuntu 17.04, Ubuntu 17.10

So I think we're clear of any regressions on the non-Debian platforms with these changes.

+1 from me, thanks Nathan and Eric.

Eric Desrochers (slashd) wrote :

Additionally, PCP upstream change the debhelper compatibility level (debian/compat) first introduced in commit : eb1ea37[1] from "5" to "9".

Hopefully this will also land in Debian soon, in order to make the change in Ubuntu pcp pkg as well.

[1] - https://github.com/performancecopilot/pcp/commit/eb1ea37 (Thu Nov 23 2017)

- Eric

Eric Desrochers (slashd) wrote :

About the pmatop segfault in 3.12.2

I just have tested 4.0.0 on Bionic and pmatop no longer segfault.

$ pmatop
pmatop: no per-disk values available

Ken McDonell (kenj) wrote :

Re systemd.

Commit a59111b in my upstream tree turns on use of systemd in PCP for all Debian-based platforms that configure thinks they're running a real systemd. This code has been heavily exercised on the non-Debian platforms for a long time, and my testing on Ubuntu 17.04 and 17.10 suggests it works just fine there, as expected.

Eric Desrochers (slashd) wrote :

3.12.2-1 from Debian got auto-synced in Ubuntu bionic

  * Default to building with Qt5 (closes: #875093)
  * Add missing Qt5 svg dependency (closes: #881649)
  * Switch from native to quilt form (closes: #881650)
  * Changing compatibility level from 5 to 9

That should cover what I have discovered, and MIR Team request so far.

The only thing missing that I can think of right now are to release the upstream fixes for which I think has been merged upstream in 4.0.0 version :

- Turning on systemd
- Fix the pmatop binary segfault

Matthias Klose (doko) wrote :

things which should be addressed:

 - Debian #805955, this is not fixed.
   You should not call dh_* in the binary-arch target without -a,
   working on the Arch: all packages as well. That should be done
   in the binary-indep target with dh_* -i calls.

 - python-six, python-json-pointer as build dependencies: Why no
   python3 equivalents? Looks like Python2 is used for the build
   by default, which is not wanted anymore for main. I'd suggest
   to replace those by their Python3 equivalents unconditionally.

 - the python3/python dependencies are wrong.
   PCP_PYTHON_PROG=python3 can be changed in the config file,
   invalidating the package dependencies. I think the python2
   alternatives should be dropped and PCP_PYTHON_PROG shouldn't
   be a config option.
   pcp Depends: python3-pcp | python-pcp.

   pcp-export-pcp2influxdb: Depends on python-requests unconditionally,
   no python3-requests.

   I assume the packages are built to be "usable" with both Python2/Python3
   but breaking package dependencies on the way. There is a reason that
   Debian and Ubuntu have separate Python2/Python3 stacks. That should
   be fixed.

 - pcp: gawk (build) dependency: looks like this one isn't needed (no gawk
   specifics), and awk is in essential.

 - lintian still reports over 200 warnings/errors, making it
   difficult to find relevant warnings. You can easily fix
   some of those.
   + Priority extra -> optional
   + unusual-interpreter: Why have /usr/bin/venv pmpython and not
     /usr/bin/pmpython? Everything else seems to be hardcoded for
     /usr/bin in the config file anyway?
   + pcp-testsuite: Please either fix (or override) the warnings
     for script-not-executable and unsual-interpreter.
   + fixing the above makes the remaining lintian warnings more
     visible. The manpage warnings might be minor (?), but
     what about the init.d-script-does-not-source-init-functions

so please fix at least the binary-indep/binary-arch targets, and the python/python3 dependency mess.

Changed in pcp (Ubuntu):
status: In Progress → Incomplete
tags: added: id-5a04ca62ac08e7d73d51f1cd
Nathan Scott (nathans) wrote :

Hi Eric, all,

| The only thing missing that I can think of right now are to release the
| upstream fixes for which I think has been merged upstream in 4.0.0 version

Just a heads up that the pcp-4.0.0 release happened a few weeks back, and
included all of Eric's fixes as well as several others related to Debian/
Ubuntu packaging for pcp (as per Matthias' list above the python deps and
binary-indep/binary-arch fixes are also included there).


To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.