[MIR] papi

[Dariusz Gadomski]

[Availability]
It's available in universe for all supported releases and architectures with
the exception of s390 - it's not supported on that architecture yet.

[Rationale]

This is a dependency of already reported main inclusion request:
pcp bug #1700827.

[Security]

No security records in cve.mitre.org, OSS security mailing list nor Ubuntu CVE tracker.

[Quality assurance]

No outstanding bugs.
44 lintian warnings according to https://<email address hidden>#papi.

No debian/watch file.

[Dependencies]

Dependencies are satisfiable in main.

[Standards compliance]

I haven't found any FHS violations.
lintian warnings mentioned in [Quality assurance] section.

[Maintenance]

No owning team yet.

[Background information]

Description of all binary packages comply to MIR.

[Dariusz Gadomski (dgadomski)]

I believe it's really useful to also include papi [1] in main as 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/

[Mathieu Trudel-Lapierre (cyphermox)]

I'm doing this review.

[Mathieu Trudel-Lapierre (cyphermox)]

- papi-examples Depends on mpi-defaults-dev, you most likely want to avoid further complicating the MIR with an additional MIR for mpi-defaults.

- there is no subscribing team.

Now, because this is a very complex package that proposes to be a performance API for software and could thus introduce an attack surface to the software it is applied to, I'd rather if there was a security review for it like there was for pcp.

I see no other issues with the package, the packaging looks to be high-quality and appropriately maintained in Debian, upstream looks responsive, etc.

[Seth Arnold (seth-arnold)]

I reviewed papi version 5.6.0-1 as checked into bionic. This should not be
considered a full security audit; in fact I stopped part-way through the
process after discovering sufficient grounds to decide that the Ubuntu
security team does not want to support this package at this time. Perhaps
with some cleanup we can re-address this package for a future cycle.

I believe there is a mistake in the Debian packaging:

- 'grep -C5 recipe' on the build log shows several failed build targets,
  but the build as a whole does not fail.

Here's some of the issues I found while reading the sources. Many of
these may not be security issues but suggest that PAPI was not intended
to be used on computers that may have malicious actors.

- xscale_iresume() buffer overflow of stack variable pmc.

- scan_prtconf() in ./src/solaris-ultra.c uses tmpnam(3) rather than
  mkstemp(3).

- _mx_init_component() in ./src/components/mx/linux-mx.c executes
  CWD-relative program if mx_counters isn't in PATH

- find_pmu_type_by_name() in ./src/libpfm4/lib/pfmlib_powerpc_perf_event.c
  may overflow its buffer if a caller provides a too-long name. Callers
  are dynamic enough that this needs its own check.

- find_pmu_type_by_name() in
  ./src/libpfm4/lib/pfmlib_intel_x86_perf_event.c
  may overflow its buffer if a caller provides a too-long name. Callers
  are dynamic enough that this needs its own check.

- find_pmu_type_by_name() in
  ./src/libpfm4/lib/pfmlib_amd64_perf_event.c
  may overflow its buffer if a caller provides a too-long name. Callers
  are dynamic enough that this needs its own check.

- pfmlib_build_event_pattrs() in ./src/libpfm4/lib/pfmlib_common.c runs
  the risk of an integer overflow in a malloc() call, calloc() would be
  safer

- main() in ./src/utils/papi_cost.c exposes an integer overflow malloc()
  call directly to the command line arguments.

- main() in ./src/utils/papi_multiplex_cost.c exposes an integer overflow
  malloc() call directly to the command line arguments.

- main() in ./src/utils/papi_command_line.c exposes an integer overflow
  malloc() call directly to the command line arguments.

- uninitialized variable 'nr' in src/perfctr-2.7.x/usr.lib/x86.c -- an
  #else #error case would be very kind

There's many cases of malloc(foo * bar); in the sources; a few I inspected
looked safe, a few weren't obvious, and some were obviously dangerous (as
above). Basically all of these should be replaced with calloc() to avoid
having to do further analysis.

There's a few cases of system() or popen() in the sources. A few of the
uses looked fine, a few looked like they deserved more hardening. I
don't think any outright need converting to execve() or libpipeline,
but I didn't inspect the callers sufficiently to be sure.

It appeared to be a recurring pattern to use sprintf() to a buffer
without checking for success; many cases did appear to constrain
referenced strings to fit within the available buffer space, but the
end result may not always be well-formed.

The full cppcheck output is 92 lines. Even after removing everything
with 'test' and 'example' and a few others, there's ~25 issues that look
like they need investigating. Here's the ones ...

[Seth Arnold (seth-arnold)]

$ grep -C5 recipe papi_5.6.0-1_amd64-2018-02-02T03\:28\:15Z.build
./src/papi_libpfm4_events.c:131: undefined reference to `pfm_get_version'
./src/papi_libpfm4_events.c:112: undefined reference to `pfm_initialize'
./src/papi_libpfm4_events.c:153: undefined reference to `pfm_strerror'
./src/papi_libpfm4_events.c:117: undefined reference to `pfm_strerror'
collect2: error: ld returned 1 exit status
Makefile:19: recipe for target 'broken_events' failed
make[2]: *** [broken_events] Error 1
make[2]: Leaving directory '/<<PKGBUILDDIR>>/src/components/perf_event/tests'
make[2]: Entering directory '/<<PKGBUILDDIR>>/src/components/perf_event_uncore/tests'
cc -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -Wextra -Wall -O2 -I. -I/usr/include -I../../.. -I../../../testlib -I../../../validation_tests -c -o perf_event_uncore.o perf_event_uncore.c
cc -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -Wextra -Wall -O2 -I../testlib -I.. -I. -c papi_native_avail.c
--
./src/papi_libpfm4_events.c:131: undefined reference to `pfm_get_version'
./src/papi_libpfm4_events.c:112: undefined reference to `pfm_initialize'
./src/papi_libpfm4_events.c:153: undefined reference to `pfm_strerror'
./src/papi_libpfm4_events.c:117: undefined reference to `pfm_strerror'
collect2: error: ld returned 1 exit status
Makefile:23: recipe for target 'perf_event_uncore' failed
make[2]: *** [perf_event_uncore] Error 1
make[2]: Leaving directory '/<<PKGBUILDDIR>>/src/components/perf_event_uncore/tests'
cc -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -Wextra -Wall -O2 -I../testlib -I.. -I. -c papi_fp_ops.c
gfortran -I../testlib -I. -I.. -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -Wextra -Wall -ffixed-line-length-132 -O1 cost.F ../testlib/libtestlib.a ../libpapi.so.5.6.0.0 -o cost -Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,-z,now -Wl,--as-needed -ldl
cc -g -O2 -fdebug-prefix-map=/<<PKGBUILDDIR>>=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g -Wextra -Wall -O2 -I../testlib -I.. -I. -c papi_xml_event_info.c
--
chmod go+rx /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi
chmod go+rx /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib
find . -perm -100 -type f -exec cp {} /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib \;
chmod go+rx /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib/*
chmod: cannot access '/<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib/*': No such file or directory
Makefile:41: recipe for target 'install' failed
make[3]: [install] Error 1 (ignored)
find . -name "*.[chaF]" -type f -exec cp {} /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib \;
cp Makefile.target /<<PKGBUILDDIR>>/debian/tmp/usr/share/papi/testlib/Makefile
make[3]: Leaving directory '/<<PKGBUILDDIR>>/src/testlib'
cd ctests; make install

[Seth Arnold (seth-arnold)]

$ cat cppcheck.txt.full
[src/components/appio/tests/appio_test_pthreads.c:128]: (error) Memory leak: callThd
[.pc/fix-typos.patch/src/sw_multiplex.c:659]: (error) Uninitialized variable: lastthread
[src/components/infiniband_umad/linux-infiniband_umad.c:242]: (error) Null pointer dereference
[src/components/libmsr/utils/libmsr_write_test.c:205]: (error) Resource leak: fileout
[src/components/perf_event_uncore/tests/perf_event_uncore.c:69]: (error) Undefined behavior: Variable 'uncore_event' is used as parameter and destination in s[n]printf().
[src/components/perfctr_ppc/ppc64_events.c:336]: (error) Memory leak: ntv_evt_info
[src/components/perfmon2/perfmon.c:601]: (error) Memory leak: newctx
[src/components/perfmon2/perfmon.c:601]: (error) Memory leak: load_args
[src/components/perfmon2/perfmon.c:757]: (error) Uninitialized variable: buf
[src/aix.c:1098]: (error) Resource leak: map_f
[src/ctests/attach2.c:129]: (error) Buffer is accessed out of bounds.
[src/ctests/attach3.c:133]: (error) Buffer is accessed out of bounds.
[src/ctests/inherit.c:59]: (error) Buffer is accessed out of bounds.
[src/ctests/mendes-alt.c:37]: (error) Uninitialized variable: t
[src/ctests/mendes-alt.c:52]: (error) Uninitialized variable: a
[src/ctests/mendes-alt.c:53]: (error) Uninitialized variable: b
[src/ctests/nineth.c:50]: (error) Uninitialized variable: num_events2
[src/ctests/omptough.c:112]: (error) Memory leak: ctrcode
[src/ctests/pthrtough2.c:88]: (error) Common realloc mistake: 'th' nulled but not freed upon failure
[src/examples/PAPI_add_remove_events.c:61]: (error) Uninitialized variable: tmp
[src/examples/PAPI_get_opt.c:21]: (error) Uninitialized variable: tmp
[src/examples/PAPI_get_real_cyc.c:16]: (error) Uninitialized variable: tmp
[src/examples/add_event/Papi_add_env_event.c:86]: (error) Uninitialized variable: b
[src/examples/PAPI_reset.c:18]: (error) Uninitialized variable: tmp
[src/examples/PAPI_set_domain.c:22]: (error) Uninitialized variable: tmp
[src/libpfm-3.y/examples_ia64_v2.0/ita2_dear.c:104]: (error) Memory leak: array
[src/libpfm-3.y/examples_ia64_v2.0/ita2_irr.c:116]: (error) Memory leak: a
[src/libpfm-3.y/examples_ia64_v2.0/ita2_irr.c:116]: (error) Memory leak: b
[src/libpfm-3.y/examples_ia64_v2.0/ita2_irr.c:116]: (error) Memory leak: c
[src/libpfm-3.y/examples_ia64_v2.0/ita_dear.c:101]: (error) Memory leak: array
[src/libpfm-3.y/examples_ia64_v2.0/ita_irr.c:118]: (error) Memory leak: a
[src/libpfm-3.y/examples_ia64_v2.0/ita_irr.c:118]: (error) Memory leak: b
[src/libpfm-3.y/examples_ia64_v2.0/ita_irr.c:118]: (error) Memory leak: c
[src/libpfm-3.y/examples_ia64_v2.0/mont_dear.c:102]: (error) Memory leak: array
[src/libpfm-3.y/examples_ia64_v2.0/mont_irr.c:116]: (error) Memory leak: a
[src/libpfm-3.y/examples_ia64_v2.0/mont_irr.c:116]: (error) Memory leak: b
[src/libpfm-3.y/examples_ia64_v2.0/mont_irr.c:116]: (error) Memory leak: c
[src/libpfm-3.y/examples_v2.x/ia64/ita2_dear.c:83]: (error) Memory leak: array
[src/libpfm-3.y/examples_v2.x/ia64/ita2_irr.c:118]: (error) Memory leak: a
[src/libpfm-3.y/examples_v2.x/ia64/ita2_irr.c:118]: (error) Memory leak: b
[src/libpfm-3.y/examples_v2.x/ia64/ita2_irr.c:118]: (error) Memory leak: c
[src/l...