Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 14 Jun 2017 16:46:49 +0000 (UTC)
From:      Andriy Gapon <avg@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-vendor@freebsd.org
Subject:   svn commit: r319951 - vendor-sys/illumos/dist/uts/common/fs/zfs
Message-ID:  <201706141646.v5EGkntI075839@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: avg
Date: Wed Jun 14 16:46:49 2017
New Revision: 319951
URL: https://svnweb.freebsd.org/changeset/base/319951

Log:
  8311 ZFS_READONLY is a little too strict
  
  illumos/illumos-gate@2889ec41c05e9ffe1890b529b3111354da325aeb
  https://github.com/illumos/illumos-gate/commit/2889ec41c05e9ffe1890b529b3111354da325aeb
  
  https://www.illumos.org/issues/8311
    Description:
    There was a misunderstanding about the enforcement details of the "Read-only"
    flag introduced for SMB/CIFS compatibility, way back in 2007 in the Sun PSARC
    2007/315 case.
    The original authors thought enforcement of the READONLY flag should work
    similarly as the IMMUTABLE flag. Unfortunately, that enforcement is
    incompatible with the expectations of Windows applications using this feature
    through the SMB service. Applications assume (and the MS File System Algorithms
    MS-FSA confirms they should) that an SMB client can:
    (a) Open an SMB handle on a file with read/write access,
    (b) Set the DOS attributes to include the READONLY flag,
    (c) continue to have write access via that handle.
    This access model is essentially the same as a Unix/POSIX application that
    creates a file (with read/write access), uses fchmod() to change the file mode
    to something not granting write access (i.e. 0444), and then continues to write
    that file using the open handle it got before the mode change.
    Currently, the SMB server works-around this problem in a way that will become
    difficult to maintain as we implement support for SMB3 persistent handles, so
    SMB depends on this fix.
    I've written a test program that can be used to demonstrate this problem, and
    added it to zfs-tests (tests/functional/acl/cifs/cifs_attr_004_pos).
    It currently fails, but will pass when this problem fixed.
    Steps to Reproduce:
      Run the test program on a ZFS file system.
    Expected Results:
      Pass
    Actual Results:
      Fail.
  
  Reviewed by: Sanjay Nadkarni <sanjay.nadkarni@nexenta.com>
  Reviewed by: Yuri Pankov <yuri.pankov@nexenta.com>
  Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
  Reviewed by: Matt Ahrens <mahrens@delphix.com>
  Reviewed by: John Kennedy <john.kennedy@delphix.com>
  Approved by: Prakash Surya <prakash.surya@delphix.com>
  Author: Gordon Ross <gwr@nexenta.com>

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

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c	Wed Jun 14 16:44:10 2017	(r319950)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_acl.c	Wed Jun 14 16:46:49 2017	(r319951)
@@ -21,7 +21,7 @@
 /*
  * Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2013 by Delphix. All rights reserved.
- * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2017 Nexenta Systems, Inc.  All rights reserved.
  */
 
 #include <sys/types.h>
@@ -2035,13 +2035,11 @@ zfs_zaccess_dataset_check(znode_t *zp, uint32_t v4_mod
 	}
 
 	/*
-	 * Only check for READONLY on non-directories.
+	 * Intentionally allow ZFS_READONLY through here.
+	 * See zfs_zaccess_common().
 	 */
 	if ((v4_mode & WRITE_MASK_DATA) &&
-	    (((ZTOV(zp)->v_type != VDIR) &&
-	    (zp->z_pflags & (ZFS_READONLY | ZFS_IMMUTABLE))) ||
-	    (ZTOV(zp)->v_type == VDIR &&
-	    (zp->z_pflags & ZFS_IMMUTABLE)))) {
+	    (zp->z_pflags & ZFS_IMMUTABLE)) {
 		return (SET_ERROR(EPERM));
 	}
 
@@ -2251,6 +2249,24 @@ zfs_zaccess_common(znode_t *zp, uint32_t v4_mode, uint
 	if (skipaclchk) {
 		*working_mode = 0;
 		return (0);
+	}
+
+	/*
+	 * Note: ZFS_READONLY represents the "DOS R/O" attribute.
+	 * When that flag is set, we should behave as if write access
+	 * were not granted by anything in the ACL.  In particular:
+	 * We _must_ allow writes after opening the file r/w, then
+	 * setting the DOS R/O attribute, and writing some more.
+	 * (Similar to how you can write after fchmod(fd, 0444).)
+	 *
+	 * Therefore ZFS_READONLY is ignored in the dataset check
+	 * above, and checked here as if part of the ACL check.
+	 * Also note: DOS R/O is ignored for directories.
+	 */
+	if ((v4_mode & WRITE_MASK_DATA) &&
+	    (ZTOV(zp)->v_type != VDIR) &&
+	    (zp->z_pflags & ZFS_READONLY)) {
+		return (SET_ERROR(EPERM));
 	}
 
 	return (zfs_zaccess_aces_check(zp, working_mode, B_FALSE, cr));

Modified: vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c
==============================================================================
--- vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c	Wed Jun 14 16:44:10 2017	(r319950)
+++ vendor-sys/illumos/dist/uts/common/fs/zfs/zfs_vnops.c	Wed Jun 14 16:46:49 2017	(r319951)
@@ -707,9 +707,11 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t 
 	}
 
 	/*
-	 * If immutable or not appending then return EPERM
+	 * If immutable or not appending then return EPERM.
+	 * Intentionally allow ZFS_READONLY through here.
+	 * See zfs_zaccess_common()
 	 */
-	if ((zp->z_pflags & (ZFS_IMMUTABLE | ZFS_READONLY)) ||
+	if ((zp->z_pflags & ZFS_IMMUTABLE) ||
 	    ((zp->z_pflags & ZFS_APPENDONLY) && !(ioflag & FAPPEND) &&
 	    (uio->uio_loffset < zp->z_size))) {
 		ZFS_EXIT(zfsvfs);
@@ -2791,10 +2793,9 @@ zfs_setattr(vnode_t *vp, vattr_t *vap, int flags, cred
 		return (SET_ERROR(EPERM));
 	}
 
-	if ((mask & AT_SIZE) && (zp->z_pflags & ZFS_READONLY)) {
-		ZFS_EXIT(zfsvfs);
-		return (SET_ERROR(EPERM));
-	}
+	/*
+	 * Note: ZFS_READONLY is handled in zfs_zaccess_common.
+	 */
 
 	/*
 	 * Verify timestamps doesn't overflow 32 bits.
@@ -4698,8 +4699,12 @@ zfs_map(vnode_t *vp, offset_t off, struct as *as, cadd
 	ZFS_ENTER(zfsvfs);
 	ZFS_VERIFY_ZP(zp);
 
+	/*
+	 * Note: ZFS_READONLY is handled in zfs_zaccess_common.
+	 */
+
 	if ((prot & PROT_WRITE) && (zp->z_pflags &
-	    (ZFS_IMMUTABLE | ZFS_READONLY | ZFS_APPENDONLY))) {
+	    (ZFS_IMMUTABLE | ZFS_APPENDONLY))) {
 		ZFS_EXIT(zfsvfs);
 		return (SET_ERROR(EPERM));
 	}



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201706141646.v5EGkntI075839>