svn commit: r302648 - vendor-sys/illumos/dist/uts/common/fs/zfs

[Andriy Gapon]

Author: avg
Date: Tue Jul 12 11:34:05 2016
New Revision: 302648
URL: https://svnweb.freebsd.org/changeset/base/302648

Log:
  7019 zfsdev_ioctl skips secpolicy when FKIOCTL is set
  
  7020 sdev_cleandir can loop forever
  
  Note that the bulk of the upstream change is not applicable to FreeBSD
  and the affected files are not even in the vendor area.
  
  illumos/illumos-gate at 45b1747515a17db45e8971501ee84a26bdff37b2
  https://github.com/illumos/illumos-gate/commit/45b1747515a17db45e8971501ee84a26bdff37b2
  
  https://www.illumos.org/issues/7019
    Currently zfsdev_ioctl, when confronted by a request with the FKIOCTL flag set,
    skips all processing of secpolicy functions. This means that ZFS is not doing
    any kind of verification of the credentials or access rights of the caller and
    assuming that (as it is an in-kernel client) all such checks have already been
    done.
    This turns out to be quite a dangerous assumption, especially with respect to
    sdev. In general I don't think it's particularly reasonable to offload this
    enforcement of access rights onto other kernel subsystems when ZFS has some
    particular local semantics in this area (delegated datasets etc) and does not
    provide any kind of API to allow other subsystems to avoid code duplication
    when doing it. ZFS should apply its normal access policy to requests from
    within the kernel, and callers should take care to give it the correct
    credentials and call it from the correct context in order to get the results
    they need.
    You can observe the currently unfortunate consequences of this bug in any non-
    global zone that has access to /dev/zvol or any subset of it via sdev profiles.
    In particular, a zone used to contain a KVM or similar which has a single zvol
    passed through to it using a <device match= block in its zone XML.
    Even though sdev makes something of an attempt to control for whether the
    caller should have access to nodes in /dev/zvol, it doesn't do this correctly,
    or really at all in the lookup call path. So, if we have a zone that's been
    given access to any part of /dev/zvol, it can simply look up the full path to
    any other zvol on the entire system, and the node will appear and be able to be
    used.
  
  https://www.illumos.org/issues/7020
    sdev_cleandir can currently hang forever when it encounters a child node that
    is busy, or when it is given a matching expr and the first entry on the list
    does not match.
    The previous code (circa 2013) iterated over the children of the node using a
    for loop with SDEV_NEXT_ENTRY, which was then changed to a while ((dv =
    SDEV_FIRST_ENTRY(ddv)) { loop. Unfortunately the continue statements that
    previously made it skip over an entry were left as they were, which now result
    in an infinite busy-loop in the kernel.
    You can trigger this pretty easily by setting up an sdev exclude rule in
    zonecfg.
    Diagnosis: look for a runaway process consuming 100% CPU in kernel -- they have
    a distinctive stack:
    # mdb -k
    > 0t1234::pid2proc | ::walk thread | ::findstack -v
    [ ffffd001efcd3310 _resume_from_idle+0x112() ]
      ffffd001efcd3360 apix_hilevel_intr_epilog+0xc1(ffffd001efcd33d0, 0)
      ffffd001efcd33c0 apix_do_interrupt+0x34a(ffffd001efcd33d0, 0)
      ffffd001efcd33d0 _sys_rtt_ints_disabled+8()
      ffffd001efcd3550 rw_enter+0x58()
      ffffd001efcd35e0 sdev_cleandir+0x60(ffffd0631b6d75d8, 0, 0)
      ffffd001efcd3630 devzvol_prunedir+0xec(ffffd0631b6d76e8)
      ffffd001efcd36d0 devzvol_readdir+0x150(ffffd06333250e00, ffffd001efcd3790,
    ffffd062dc990e18, ffffd001efcd37dc, 0, 0)
      ffffd001efcd3760 fop_readdir+0x6b(ffffd06333250e00, ffffd001efcd3790,
    ffffd062dc990e18, ffffd001efcd37dc, 0, 0)
      ffffd001efcd3830 walk_dir+0xee(ffffd06333250e00, ffffd0669e4483c8,
    fffffffffbbdf410)
      ffffd001efcd3850 prof_make_names_walk+0x2e(ffffd0669e4483c8,
    fffffffffbbdf410)
      ffffd001efcd38b0 prof_make_names+0xfc(ffffd0669e4483c8)
  
  Reviewed by: Robert Mustacchi <rm at joyent.com>
  Reviewed by: Richard Lowe <richlowe at richlowe.net>
  Reviewed by: Matthew Ahrens <mahrens at delphix.com>
  Approved by: Dan McDonald <danmcd at omniti.com>
  Author: Alex Wilson <alex.wilson at joyent.com>

Modified:
  vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c	Tue Jul 12 11:29:19 2016	(r302647)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_ioctl.c	Tue Jul 12 11:34:05 2016	(r302648)
@@ -25,7 +25,7 @@
  * Portions Copyright 2011 Martin Matuska
  * Copyright 2015, OmniTI Computer Consulting, Inc. All rights reserved.
  * Copyright 2015 Nexenta Systems, Inc.  All rights reserved.
- * Copyright (c) 2014, Joyent, Inc. All rights reserved.
+ * Copyright (c) 2014, 2016 Joyent, Inc. All rights reserved.
  * Copyright (c) 2011, 2015 by Delphix. All rights reserved.
  * Copyright (c) 2013 by Saso Kiselkov. All rights reserved.
  * Copyright (c) 2013 Steven Hartland. All rights reserved.
@@ -6022,7 +6022,7 @@ zfsdev_ioctl(dev_t dev, int cmd, intptr_
 	}
 
 
-	if (error == 0 && !(flag & FKIOCTL))
+	if (error == 0)
 		error = vec->zvec_secpolicy(zc, innvl, cr);
 
 	if (error != 0)