I reviewed zfs-linux version 0.6.5.4-0ubuntu2 as checked into xenial.
This should not be considered a full security audit but rather a gauge of
maintainability.

- zfs is a storage system with extensive features
- Build-Depends: autotools-dev, autoconf, autogen, automake, debhelper,
  dh-autoreconf, dh-systemd, dkms, libselinux1-dev, libtool, uuid-dev,
  zlib1g-dev,
- Is the dkms build-depend still necessary?
- Many lintian errors, warnings:
  - package-name-doesnt-match-sonames
  - postinst-must-call-ldconfig
  - debian-changelog-line-too-long
  - init.d-script-not-marked-as-conffile
  - init.d-script-not-included-in-package
  - systemd-no-service-for-init-script
- Does not (yet) do encryption
- Does not itself do networking, though zfs send | zfs receive often
  operate over e.g. ssh
- Provides kernel modules, zed event daemon, utilities, test tools
- zed event daemon spawns 'zedlets' on specific events; care must be taken
  when writing eventlets to ensure they properly terminate.
- zed has nice daemonization routines to send error messages to the parent
  process
- pre/post inst/rm scripts are automatically generated
- init scripts / systemd units discover devices in storage pools, import
  pools, mount filesystems, create zvol block devices, start zed, export
  any zfs shared filesystems via nfs or cifs, etc.
- No dbus services
- No setuid executables
- programs in /sbin/:
  - zpios
  - zhack
  - zfs
  - zinject
  - fsck.zfs
  - ztest
  - mount.zfs
  - zdb
  - zstreamdump
  - zpool
  /usr/sbin/:
  - zed
- The /etc/sudoers.d/zfs file is entirely commented out; it suggests some
  rules that can be used to allow passwordless access to the read-only zfs
  and zpool subcommands. However, this file also includes some rules to
  allow members of adm, admin, or staff groups access to full root
  privileges. This is not what the comment at the top of the file suggests
  and I see no reason for this file to contain these rules, even if
  commented out.
- Extensive udev rules to create zvol block devices and /dev/disk/by-vdev/
  for more-complicated vdev configurations
- No tests run during build
- Good selection of kernel smoke tests provided in debian/tests/
- There are no provided cron jobs but system administrators would be wise
  to schedule pool scrubs frequently; typically weekly or monthly is
  recommended based on drive quality, level of redundancy in the pool,
  data read rate and working set size.
- More than the usual number of warnings from libtool

- Some process spawning: some via zed, some via the kernel, some via
  utilities. They looked careful.
- Memory management looked careful
- Not much file IO in the usual sense
- Logging looked careful, very extensive error reporting
- A lot of environment variable use, some could have unexpected
  consequences
- Extensive privileged commands; far too many to carefully audit
- No cryptography (yet)
- Does not itself do networking
- There are extensive privileged portions of code, including kernel
  modules; some privilege checks that make sense in OpenSolaris are
  stubbed out and ignored in Linux, we should investigate further.
- Minimal use of temp files, handled carefully
- Does not use WebKit
- Minor errors reported by cppcheck
- extensive shellcheck warnings on many of the scripts; I don't think any
  are used in deployed systems
- Does not use PolicyKit

The ZFS code is professionally written: errors are checked extensively,
comments are tasteful, functions are just the right size, and the design
feels clean to the point of obviousness. The finer points are of course
very complicated and we'll need to rely upon upstream for help with
anything non-trivial.

I did not take sufficient time to fully evaluate the kernel implementation:

- large amount of security engine in zfs:

  - zfs_dozonecheck_impl() for example checks if the currently executing
    task is executing in the global zone or in a local zone. INGLOBALZONE
    appears to be defined to (1) making the check useless on Linux. This
    should probably be extended to perform similar Linux container checks.

  - Most OpenZFS implementations support NFS4 ACLs; I believe zfsonlinux
    supports POSIX ACLs instead. It looks like there may be support for
    both in the codebase, or perhaps framework to support both styles.

  - Some other ZFS implementations allow delegating authority over
    datasets to specific uids -- I understand this is entirely missing on
    Linux but it feels like this deserves investigation.

  - Extensive amount of API is controlled via ioctl; some amount of API
    may be exposed to less-privileged users via .zfs/ directories. Both
    deserve closer inspections.

We must validate that unprivileged containers cannot manipulate pools
or datasets.

I took some notes while reading that I hope are useful:

- the libraries install directly to /lib, is this intentional?
- efi_use_whole_disk() leaks &efi_label via VT_ENOSPC error return
- efi_get_info() hardcodes device names and name schemes in /dev/:
  sd hd md vd xvd zd dm- ram loop
- nfs_check_exportfs() hardcodes /usr/sbin/exportfs
- ztest builds in some assumptions about pathnames, safety of
  /proc/pid/exe, etc. It's not a general purpose utility but may still be
  useful enough to continue packaging. It's only suitable for test
  systems.
- vdev_elevator_switch() broken strncmp() test
- _finish_daemonize() if devnull is 0, 1, 2, the close(devnull) call will
  leave the program without that fd
- zfs_fuid_node_add() modifies zfs_fuid_info_t list without obvious
  locking -- are calls to this routine suitably locked elsewhere?
- ddt_zap_update() allocates cbuf[sizeof(dde_phys)] on the stack -- which
  is an array DDT_PHYS_TYPES long of structs which themselves have an
  embedded array of structs with further embedded array of integers. How
  likely is it for this to blow out the stack depth?
- zpool_vdev_remove() includes misleading error message that top-level
  devices can be removed
- zpool_vdev_remove() awkward error message "pool must be upgrade to
  support log removal"

Environment variables used:
ZINJECT_DEBUG
ZFS_ABORT
ZPOOL_IMPORT_PATH
SPA_CONFIG_PATH
ZFS_STACK_SIZE
ZFS_DEBUG
TZ
UU_DIE_ABORTS
ZFS_MODULE_LOADING
ZFS_MODULE_TIMEOUT
__ZFS_POOL_RESTRICT
ZTEST_FD_DATA
LD_LIBRARY_PATH

Please remove the %adm %admin %staff section of /etc/sudoers.d/zfs.

Please investigate the lintian messages and libtool warnings.

Security team ACK for promoting zfs-linux to main is conditional upon
the sudoers file; please ensure that is addressed before 16.04 LTS is
released. (There's no need to fix 15.10 packages and no need to get
a CVE.)

Thanks