Comment 21 for bug 1853506

I reviewed ndctl as checked into focal. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

ndctl is comprised of utilities and libraries for managing the libnvdimm (non-volatile memory device) sub-system in the Linux kernel

- No CVEs readily found.
  Gleaned the git repository, https://github.com/pmem/ndctl. Appears to be actively maintained.
  Security-wise, noted fixes for a memory leak and non-null terminated strings.
- Build-Depends: debhelper-compat (= 12), pkg-config, libkmod-dev, libudev-dev, uuid-dev,
  libjson-c-dev, bash-completion, systemd, libkeyutils-dev, asciidoctor
- No pre/post inst/rm scripts.
- There is an init script, debian/ndctl.init that is is installed as /etc/init.d/ndctl-monitor.
  All actions are circumvented to systemctl.
- There is a systemd unit file, ndctl-monitor.service, for the ndctl monitor daemon. The daemon
  catches smart events notify from firmware and outputs the notifications (in json format) to a
  logfile.
- No dbus services.
- No setuid binaries.
- 2 binaries, ndctl and daxctl in /usr/bin
- No sudo fragments.
- No udev rules.
- There are unit-tests and autopkgtests. The unit tests were skipped. There has been considerable
  discussion in this bugreport about providing regression testing.
- No cron jobs.
- Build reported following...
  - configure: WARNING: unrecognized options: --disable-maintainer-mode
  - quite a few alignment warnings for "address-of-packed-member",
    i.e.,
nfit.c: In function ‘ndctl_bus_cmd_new_translate_spa’:
nfit.c:65:25: warning: taking address of packed member of ‘struct nd_cmd_translate_spa’ may result in an unaligned pointer value [-Waddress-of-packed-member]
   65 | cmd->firmware_status = &translate_spa->status;
      | ^~~~~~~~~~~~~~~~~~~~~~

  - following lintian warnings,
    - malformed-deb-archive newer compressed control.tar.xz
    - init.d-script-uses-usr-interpreter etc/init.d/ndctl-monitor /usr/bin/env
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor start
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor stop
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor restart
E: ndctl: init.d-script-does-not-implement-required-option etc/init.d/ndctl-monitor force-reload
W: ndctl: unusual-interpreter etc/init.d/ndctl-monitor #!/lib/init/init-d-script
W: ndctl: init.d-script-does-not-source-init-functions etc/init.d/ndctl-monitor

   - following dpkg warnings
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/daxctl/usr/bin/daxctl was not linked against libndctl.so.6 (it uses none of the library's symbols)
dpkg-shlibdeps: warning: package could avoid a useless dependency if debian/daxctl/usr/bin/daxctl was not linked against libuuid.so.1 (it uses none of the library's symbols)

- execlp() called without an absolute path to bring up help pages. A call to "kfmclient" and
  once to call "man".
- Inspecting a random sampling of memory mgmt routines, the memory allocation looked good;
  memcpy() ok; none of the sprintf() nor asprintf() checked return value.
- File IO looked ok.
- Logging looked ok. We do not --enable-debug so limited debugging available.
  -daxctl_set_log_fn allows user to write custom function to override default!
-There are several environment vars. Could not readily find documentation on any of them.
   - log_env overrides log priority set in config file but uses secure_logenv so probably ok.
   - code does getenv("MANPATH"); then calls setenv("MANPATH") with gotten value. Seems bad idea.
- ioctls looked ok.
- Cryptography: looks ok.
  ndctl-setup|update|remove-passphrase uses the kernel keyring to enable
  a security passphrase for NVDIMM(s).
  binary blobs of the encrypted masterkey and NVDIMM passphrase(s) are
  stored in /etc/ndctl/keys directory and loaded into memory and
  compared (in a way validated) with kernel keyring with ndctl command.
- a single testcase uses hard-coded tmp file but this testcase is skipped.
- No WebKit.
- No PolicyKit.
- There were some cppcheck results, upon closer examination they seem ok.
[ndctl/check.c:1150]: (error) Signed integer overflow for expression '(549755813888)-4096'.
[ndctl/dimm.c:1216]: (error) Memory leak: actx.f_out
[util/json.c:871]: (error) Uninitialized variable: raw_uuid
[ndctl/lib/libndctl.c:5577]: (error) Uninitialized variable: uuid
[ndctl/lib/libndctl.c:5578]: (error) Uninitialized variable: uuid

- Quite a few scripts in test directory reported following warning,
"Double quote to prevent globbing and word splitting"

GENERAL COMMENTS

- There are other licenses besides GPL licences.

- Note: opened an issue upstream about the unaligned pointer warning from compiler, https://github.com/pmem/ndctl/issues/131

Security team ACK only on condition that regression tests are available.