[MIR] pmdk

Bug #1790856 reported by Christian Ehrhardt  on 2018-09-05
20
This bug affects 2 people
Affects Status Importance Assigned to Milestone
pmdk (Ubuntu)
Undecided
Andreas Hasenack

Bug Description

Availability: PMDK is rather new (Cosmic) and builds for amd64 (main purpose)
There also is arm code in the lib, but lacking any request as well as arm-nvdimm HW it isn't built there.

Rationale:
- There is a request to enhance qemu nvdimm support (bug 1745900) which is reasonable looking forward to the potential future with nvdimms.
- It would not be useful to a "large" part of the user base as nvdimms will be rare for a while. But huge installations might converge to them for some cases soon.
- The mentioned qemu change would make qemu (main) require libpmem1 (currently universe).

Security:
- there are no CVEs yet to prove how the projects will behave by looking at the past
- The project is rather active in general
  => http://pmem.io/
  => https://github.com/pmem/pmdk/pulse

Quality assurance:
- The package builds helper tools as well as a lib "libpmem1". Both work out-of the box (if you have the HW below for more)
- no debconf questions
- Due to beign so recent we have no good insight on potential long term bugs, obviously thre are none atm.
- Bug trackers are clear, but mostly due to being new
=> https://bugs.launchpad.net/ubuntu/+source/pmdk
(Well maintained in Ubuntu; not in Debian)
- the nature of this package is to support nvdimms which are still rare HW, so not everybody can easily test these things completely. IMHO to some extend we rely on our partners asking for these features.
- Unit tests exist and run on build
- debian/watch is in place
- no obsolete or demoted packages in the dependencies

UI standards:
- not a UI package

Dependencies:
- The libs don't have a lot of runtime dependencies and for now it seems we only want to pull in libpmem1 (not all the others, but that might change with more adoption of that code)
- The associated pmdk-tools would imply further MIRs on src:libfabric and src:ndctl which we are currently not intending to push

Standards compliance:
- follows FHS rules
- is under rather new standards 4.1.5

Maintenance:
- maintained by the server Team (atm in universe)
- ownign team would be the ubuntu-server Team

Background information:
persistent memory can be hard, this library abstracts special differences in CPUs/Architectures/devices and provides a single API to handle those.
Applications that want to use pmem/nvdimm should use PMDK instead of re-implementing things everywhere with the same mistakes.

Related branches

To add one more reason to do this like "now" (=early 19.04) from my personal POV.
While rare today, looking into the future I'd think the availability of nvdimms will rise.

So looking forward to 20.04 we have multiple options:
- nvdimms are a thing by then, so we better start in 19.04 to get things right until 20.04
- nvdimms are a thing by then but pmdk is crap, we realize that and can take it out of main before 20.04
- nvdimms are a thing by then and get a huge boost of interest, we better start in 19.04 to be one of the few supporting it and due to that get some of that "attention" towards Ubuntu
- nvdimms are identified to be crap, we realize that and can take it out of main before 20.04
...

This goes on, but for all of the thoughts that I had starting this in 19.04 would be the best option for our way to a great 20.04.

Matthias Klose (doko) wrote :

some comments:

 - the package descriptions mention the ABIs as experimental. How do you want
   to handle that in main, it's not ideal.
- I don't like the idea of only building that for amd64. Please could you build
  that everywhere, and see where it fails? This doesn't hide the fact that the
  package is not ported to some archs, and also builds on architectures where
  it is not yet fully supported. You could limit then the build dependencies in qemu
  to the architectures you want to support.
- please have a look at the bundled libraries in src
  at least jemalloc is packaged in Ubuntu, I didn't check
  for the other libraries.

Changed in pmdk (Ubuntu):
status: New → Incomplete
Andreas Hasenack (ahasenack) wrote :

Regarding jemalloc, I asked about that point exactly before submitting this package for archive inclusion:

https://bugs.launchpad.net/ubuntu/+bug/1752378/comments/69

Upstream's answer was:
https://bugs.launchpad.net/ubuntu/+bug/1752378/comments/72

I will ask upstream about non-amd64 support. There is this note in their github page about arm:
https://github.com/pmem/pmdk#experimental-support-for-64-bit-arm

Andreas Hasenack (ahasenack) wrote :

Only pmemcto is flagged as experimental: http://pmem.io/pmdk/libpmemcto/

If qemu doesn't link with it, directly or indirectly, this particular binary package could remain in universe.

On Fri, Sep 14, 2018 at 4:21 PM Andreas Hasenack <email address hidden>
wrote:

> Only pmemcto is flagged as experimental: http://pmem.io/pmdk/libpmemcto/
>
> If qemu doesn't link with it, directly or indirectly, this particular
> binary package could remain in universe.
>

pmemcto is not a binary needed by qemu for what we had in mind.

Andreas Hasenack (ahasenack) wrote :

I got confirmation from upstream @intel that pmdk is amd64 only, and there is some experimental arm64 code:
"""
There's no official support for architectures other than x86_64, because PMDK requires per-architecture code (see src/libpmem/$(ARCH)/).

Aarch64-specific code exists in the tree (it was provided by Intel employee for some experimental project and then refactored into separate directory by me, so it shouldn't break with each libpmem change), but nobody stepped-in to maintain it and verify that it actually works.
I know that this code a) doesn't use non-invalidating flushing (the code exists, but there's no detection code, so it's not wired up) and b) doesn't provide aarch64-optimized versions of memcpy/memmove/memset, which means that the code is very likely slow.
Until there's a maintainer for aarch64, bugs specific to that architecture will not block the release.
"""

Matthias Klose (doko) wrote :

ok, then pmemcto should stay in universe.

The package is still missing a bug scubscriber.

Joshua Powers (powersj) wrote :

ubuntu-server is now subscribed to the bugs for pmdk

marking as 'new'

Changed in pmdk (Ubuntu):
status: Incomplete → New

The feedback so far was answered and seems ok:
- ABIs experimental -> only pmemcto which will not get into MAIN
- amd64-only -> clarified with upstream really is only amd64 only atm
- bundled jemalloc -> explained by upstream has to stay bundled
- package subscription -> done by powersj in comment #8

Status is back to new, the security Team has it queued AFAIK.
Is there another MIR Team Task we (I?) could do or should we flip the status from new to whatever is applicable reflect that next step is the Security team?

Matthias Klose (doko) on 2018-09-25
Changed in pmdk (Ubuntu):
assignee: nobody → Ubuntu Security Team (ubuntu-security)
Adam Borowski (kilobyte) wrote :

PMDK is since recently in Debian, maintained by me. Version 1.5 (Ubuntu still has only 1.4).

Real architecture support is limited to amd64. The nature of this software (mmapping hundreds of GB -- or terabytes -- of pmem) means i386 and armhf are outright out. Upstream support for arm64 is unmaintained and has mostly bitrotten; as far as I know there's no hardware capable of using nvdimms, at least as persistent mode is concerned (it might be possible to use the dimms in volatile mode?). I asked riscv64 people, there are no cache control instructions in the ISA and no platforms with unstandardized support can flush all layers of the cache -- thus even anticipatory porting of PMDK to riscv64 is currently impossible. I have no idea about ppc64el but at least ndctl is rumoured to work -- no one ported PMDK yet, though. Wrong-endian (s390x) would probably require substantial work.

libpmemcto has became even more experimental and found a nice home in /dev/null. libpmemobj-cpp has been split out as a separate package; that part is not very advanced thus you might skip it for now.

There's also pmem-convert for upgrades between major versions; its packaging will be complex but you'd want that part once I'm done with it.

Launchpad Janitor (janitor) wrote :

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

Changed in pmdk (Ubuntu):
status: New → Confirmed
Andreas Hasenack (ahasenack) wrote :

I'm working on syncing with debian's 1.5. I might first update ours to 1.5, and then merge.

Andreas Hasenack (ahasenack) wrote :

The pmdk 1.5 update is needed to unblock https://launchpad.net/ubuntu/+source/libpmemobj-cpp in disco-proposed

Andreas Hasenack (ahasenack) wrote :

Just hit a snag with the new 1.5 upstream version, which I now realize Adam mentioned earlier.

It changed the ondisk format[1] and requires an external tool[2] to convert to the new format. This tool (pmdk-convert) is not packaged yet. If we upgrade to 1.5, we would leave 1.4 users with no way to convert to the 1.5 format.

That being said, I believe the version that should be evaluated for an MIR is at least 1.5, due to the changes it introduces.

1. https://github.com/pmem/pmdk/blob/master/ChangeLog#L9
2. https://github.com/pmem/pmdk-convert

Since nothing happened for a while here an interim update, atm this seems to be on track for security review on time to make it into Ubuntu 20.04.

Adam Borowski (kilobyte) wrote :

The hardware is commercially available for more than half a year, thus proper support is more urgent than it was when this bug was initially filed. At this time, the DIMMs work only with large servery machines, for which large numbers of qemu VMs is one of most widespread uses. Thus, it's important that qemu can use NVDIMMs in non-volatile modes.

The jemalloc issue: it's not used by libpmem{1,-dev} which is the part you'd want to promote to main first. Also, it's gone in the version that will be released in January/February.

As for other architectures: arm64 has been fixed and passes tests (although there's AFAIK still no hardware), ppc64el is being worked on.

If you want to promote only libpmem for now, pmdk-convert is not required.

On the other hand, the dependency on ndctl (both libndctl and libdaxctl, built by that source) is now effectively mandatory.

Download full text (3.2 KiB)

I reviewed pmdk 1.7-1ubuntu1 as checked into focal. This shouldn't be
considered a full audit but rather a quick gauge of maintainability.

pmdk comes from Persistent Memory Development Kit and it's a collection of
libraries and tool which allows applications to access persistent memory as
memory-mapped files.

- No CVEs found.
- No encryption dependency in particular.
- No pre/post inst/rm scripts
- No init scripts.
- No systemd units.
- No dbus services.
- No setuid binaries.
- binaries in PATH:
  - ./usr/bin/daxio (pmdk-tools)
  - ./usr/bin/pmempool (pmdk-tools)
  - ./usr/bin/rpmemd (pmdk-tools)
- No sudo fragments found.
- No udev rules.
- 262 unit tests present.
- No autopkgtests.
- No cronjobs.
- Build logs
  - Minor warnings during the build:
    - install: WARNING: ignoring --strip-program option as -s option was not specified
    - dpkg-gencontrol: warning: Depends field of package libpmemlog-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libvmmalloc-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libvmem-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libpmemblk-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libpmem-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package librpmem-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libpmempool-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
    - dpkg-gencontrol: warning: Depends field of package libpmemobj-dev:
      substitution variable ${shlibs:Depends} used, but is not defined
- No relevant process spawning found.
- Memory management seems to be done properly. The project have its own
  implementation of memcpy and memset to be used by the user. No problems
  found on this part.
- No issues with log found.
- Those environment variable were found:
  - NON_PMEM_FS_DIR
  - PMEM_FS_DIR
  - PMEM_IS_PMEM_FORCE
  - PMEM_MMAP_HINT
  - PMEM_MOVNT_THRESHOLD
  - PMEM_NO_CLFLUSHOPT
  - PMEM_NO_CLWB
  - PMEM_NO_FLUSH
  - PMEM_NO_MOVNT
  - PMREORDER_EMIT_LOG
  - PMREORDER_MARKER_NAME
  - RPMEM_CMD
  - RPMEM_ENABLE_SOCKETS
  - RPMEM_ENABLE_VERBS
  - RPMEM_MAX_NLANES
  - RPMEM_SSH
  - RPMEM_WORK_QUEUE_SIZE
  - VMMALLOC_FORK
  - VMMALLOC_POOL_DIR
  - VMMALLOC_POOL_SIZE
- No use of privileged functions.
- No use of cryptography.
- Use of temp files looks safe. Most of them for tests.
- No use of networking.
- No use of WebKit.
- No use of PolicyKit.

- cppcheck shows multiple uninitialized variable cases. Nothing concerning.

No serious issues found in coverity results. Upstream also has a coverity
instance and they do a good job tracking and fixing the found issues.
Most of the issues are in examples and tests.

Security team ACK for promoting pmdk to main only for amd64 since...

Read more...

Adam Borowski (kilobyte) wrote :

Thanks for your review. There's next release coming probably this week; -rc1 is already out. The changes relevant for MIR are:
 * vmem and vmmalloc are dropped (moved to another source, but it's not main material).
   These were the only parts doing custom memory management in DRAM.
 * ppc64el support has been added

Having rpmem in a different component would require splitting the pmdk-tools package and putting rpmemd into a separate binary.

Andreas Hasenack (ahasenack) wrote :

The split is doable. What hurts, though, is having these kind of changes (like vmem and vmalloc moving to another source) still happening. We don't expect a package in main to still be going through these types of big changes. Has this stabilized?

bug 1853506 and bug 1790856 are ready (process-wise) when you are @ahasenack.
As checked on IRC, let me know when all is ready to add the dependency pulling it in.

Changed in pmdk (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → Andreas Hasenack (ahasenack)
status: Confirmed → In Progress
Andreas Hasenack (ahasenack) wrote :

Splitting will serve another purpose too. rpmemd also links with libfabric, which is in universe. By moving rpmemd out of pmdk-tools, and into a new universe package, we won't have to MIR libfabric.

Adam Borowski (kilobyte) wrote :

Version 1.8 has just been released. As I mentioned before, besides splitting out deprecated and out of scope for PMDK libvmem and libvmmalloc, support for ppc64el has been added, thanks to Lucas Magalhães of IBM.

While the ppc64el port is marked as experimental, this is mostly due to higher-level libraries being untrustworthy (they do pass the testsuite, but are way too new and complex). As for low-level libpmem, the ppc specific code is short and looks obviously correct. It is also what current external projects want (qemu, fio, ceph). Especially because of qemu, I believe you'd want libpmem{1,-dev}:ppc64el in main.

This seems to auto-inclue way too much:
From
https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.html

Rescued from pmdk (Uploader: ahasenack), libpmemblk-dev, libpmemblk1-debug, libpmemlog-dev, libpmemobj-dev, libpmempool-dev, librpmem-dev, libvmem-dev, libvmmalloc-dev

Let us add those to extra-excludes to only pull in libpmem1 itself and libpmem-dev.

@Andreas - take a look at these and ack+push if you are ok with that:
=> https://code.launchpad.net/~paelzer/ubuntu-seeds/+git/ubuntu/+merge/378435

Adam Borowski (kilobyte) wrote :

Sorry if I was unclear: libpmemobj and friends are well-tested on amd64, it's only the ppc64el port of those that's very new (mostly because of many hardcoded x86 assumptions, like page and especially cacheline size, that are untrue on ppc). The library has gone through six years of development already, thus can be considered pretty solid.

pmempool is a tool (and library) for managing libpmemobj pools.

The rest (pmemblk and pmemlog) are somewhat less useful but still are a part of the product.

Thus, I'd recommend skipping only parts that are experimental: rpmem on amd64, and anything but perhaps libpmem on arm64 and ppc64el. The debug libs belong in universe, too, I guess.

Thanks for explaining more details and backgrounds Adam!
But promotion to main isn't really done per-architecture.

That only exists via e.g. having a package build only x86 binaries and then promoting it.
I've not yet seen a package building x86,arm64,ppc64 binaries but then promoting only some of the architectures.

Therefore I suggest for now lets push src:pmdk and binary:libpmem1 and binary:libpmem1-dev to main, but leave the others in universe for now.

That will not break the back of the "middle-tier-libs" like pmempool, libpmemobj, ...
The will still be in a package that has it's source in main being monitored and cared for - just a slightly less hard-commitment.
To me it seems to be the cleanest cut we can make right now.

My MP that I linked above would achieve exactly that.

Sebastien Bacher (seb128) wrote :

$ ./change-override -c main -t pmdk
Override component to main
pmdk 1.7-1ubuntu1 in focal: universe/libs -> main
Override [y|N]? y
1 publication overridden.

$ ./change-override -c main libpmem1 libpmem-dev
Override component to main
libpmem1 1.7-1ubuntu1 in focal amd64: universe/libs/optional/100% -> main
libpmem1 1.7-1ubuntu1 in focal arm64: universe/libs/optional/100% -> main
libpmem-dev 1.7-1ubuntu1 in focal amd64: universe/libdevel/optional/100% -> main
libpmem-dev 1.7-1ubuntu1 in focal arm64: universe/libdevel/optional/100% -> main
Override [y|N]? y
4 publications overridden.

Changed in pmdk (Ubuntu):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers