From 2889ec41c05e9ffe1890b529b3111354da325aeb Mon Sep 17 00:00:00 2001 From: Gordon Ross Date: Thu, 23 Mar 2017 19:58:29 -0400 Subject: [PATCH] 8311 ZFS_READONLY is a little too strict Reviewed by: Sanjay Nadkarni Reviewed by: Yuri Pankov Reviewed by: Andrew Stormont Reviewed by: Matt Ahrens Reviewed by: John Kennedy Approved by: Prakash Surya --- usr/src/pkg/manifests/system-test-zfstest.mf | 2 + usr/src/test/zfs-tests/cmd/dos_ro/Makefile | 22 +++ usr/src/test/zfs-tests/cmd/dos_ro/dos_ro.c | 143 ++++++++++++++++++ usr/src/test/zfs-tests/include/commands.cfg | 1 + usr/src/test/zfs-tests/runfiles/delphix.run | 3 +- usr/src/test/zfs-tests/runfiles/omnios.run | 3 +- .../test/zfs-tests/runfiles/openindiana.run | 3 +- .../functional/acl/cifs/cifs_attr_003_pos.ksh | 14 +- .../functional/acl/cifs/cifs_attr_004_pos.ksh | 141 +++++++++++++++++ usr/src/uts/common/fs/zfs/zfs_acl.c | 28 +++- usr/src/uts/common/fs/zfs/zfs_vnops.c | 19 ++- 11 files changed, 360 insertions(+), 19 deletions(-) create mode 100644 usr/src/test/zfs-tests/cmd/dos_ro/Makefile create mode 100644 usr/src/test/zfs-tests/cmd/dos_ro/dos_ro.c create mode 100644 usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_004_pos.ksh diff --git a/usr/src/pkg/manifests/system-test-zfstest.mf b/usr/src/pkg/manifests/system-test-zfstest.mf index 7d2637a592cf..62cc59b17a2a 100644 --- a/usr/src/pkg/manifests/system-test-zfstest.mf +++ b/usr/src/pkg/manifests/system-test-zfstest.mf @@ -154,6 +154,7 @@ file path=opt/zfs-tests/README mode=0444 file path=opt/zfs-tests/bin/chg_usr_exec mode=0555 file path=opt/zfs-tests/bin/devname2devid mode=0555 file path=opt/zfs-tests/bin/dir_rd_update mode=0555 +file path=opt/zfs-tests/bin/dos_ro mode=0555 file path=opt/zfs-tests/bin/file_check mode=0555 file path=opt/zfs-tests/bin/file_trunc mode=0555 file path=opt/zfs-tests/bin/file_write mode=0555 @@ -186,6 +187,7 @@ file path=opt/zfs-tests/tests/functional/acl/cifs/cifs.kshlib mode=0444 file path=opt/zfs-tests/tests/functional/acl/cifs/cifs_attr_001_pos mode=0555 file path=opt/zfs-tests/tests/functional/acl/cifs/cifs_attr_002_pos mode=0555 file path=opt/zfs-tests/tests/functional/acl/cifs/cifs_attr_003_pos mode=0555 +file path=opt/zfs-tests/tests/functional/acl/cifs/cifs_attr_004_pos mode=0555 file path=opt/zfs-tests/tests/functional/acl/cifs/cleanup mode=0555 file path=opt/zfs-tests/tests/functional/acl/cifs/setup mode=0555 file path=opt/zfs-tests/tests/functional/acl/nontrivial/cleanup mode=0555 diff --git a/usr/src/test/zfs-tests/cmd/dos_ro/Makefile b/usr/src/test/zfs-tests/cmd/dos_ro/Makefile new file mode 100644 index 000000000000..642de01e599e --- /dev/null +++ b/usr/src/test/zfs-tests/cmd/dos_ro/Makefile @@ -0,0 +1,22 @@ +# +# This file and its contents are supplied under the terms of the +# Common Development and Distribution License ("CDDL"), version 1.0. +# You may only use this file in accordance with the terms of version +# 1.0 of the CDDL. +# +# A full copy of the text of the CDDL should have accompanied this +# source. A copy of the CDDL is also available via the Internet at +# http://www.illumos.org/license/CDDL. +# + +# +# Copyright 2017 Nexenta Systems, Inc. All rights reserved. +# + +PROG = dos_ro + +include $(SRC)/cmd/Makefile.cmd + +LDLIBS += -lnvpair + +include ../Makefile.subdirs diff --git a/usr/src/test/zfs-tests/cmd/dos_ro/dos_ro.c b/usr/src/test/zfs-tests/cmd/dos_ro/dos_ro.c new file mode 100644 index 000000000000..4f0510f72bd4 --- /dev/null +++ b/usr/src/test/zfs-tests/cmd/dos_ro/dos_ro.c @@ -0,0 +1,143 @@ +/* + * This file and its contents are supplied under the terms of the + * Common Development and Distribution License ("CDDL"), version 1.0. + * You may only use this file in accordance with the terms of version + * 1.0 of the CDDL. + * + * A full copy of the text of the CDDL should have accompanied this + * source. A copy of the CDDL is also available via the Internet at + * http://www.illumos.org/license/CDDL. + */ + +/* + * Copyright 2017 Nexenta Systems, Inc. All rights reserved. + */ + +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +extern const char *__progname; + +int vflag = 0; + +static int +dosattr_set_ro(int fildes, const char *fname) +{ + nvlist_t *nvl = NULL; + int err; + + err = nvlist_alloc(&nvl, NV_UNIQUE_NAME, 0); + if (err != 0) + return (err); + + (void) nvlist_add_boolean_value(nvl, A_READONLY, 1); + + if (fname == NULL) { + err = fsetattr(fildes, XATTR_VIEW_READWRITE, nvl); + } else { + err = setattrat(fildes, XATTR_VIEW_READWRITE, fname, nvl); + } + if (err < 0) { + err = errno; + if (vflag > 1) { + (void) fprintf(stderr, + "dosattr_set: setattrat (%s), err %d\n", + fname, err); + } + } + + nvlist_free(nvl); + + return (err); +} + +void +usage(void) +{ + (void) fprintf(stderr, "usage: %s [-v] file\n", + __progname); + exit(1); +} + +int +main(int argc, char **argv) +{ + char *fname; + int c, fd, n; + + while ((c = getopt(argc, argv, "v")) != -1) { + switch (c) { + case 'v': + vflag++; + break; + case '?': + default: + usage(); + break; + } + } + + if (optind + 1 != argc) + usage(); + fname = argv[optind]; + + fd = open(fname, O_CREAT | O_RDWR, 0644); + if (fd < 0) { + perror(fname); + exit(1); + } + + if (vflag) + (void) fprintf(stderr, "Write 1 (mode 644)\n"); + n = write(fd, "mode 644 OK\n", 12); + if (n != 12) { + (void) fprintf(stderr, "write mode 644, err=%d\n", errno); + exit(1); + } + + if (vflag) + (void) fprintf(stderr, "Chmod 444\n"); + n = fchmod(fd, 0444); + if (n < 0) { + (void) fprintf(stderr, "chmod 444, err=%d\n", errno); + exit(1); + } + + if (vflag) + (void) fprintf(stderr, "Write 2 (mode 444)\n"); + n = write(fd, "mode 444 OK\n", 12); + if (n != 12) { + (void) fprintf(stderr, "write mode 444, err=%d\n", errno); + exit(1); + } + + if (vflag) + (void) fprintf(stderr, "Set DOS R/O\n"); + n = dosattr_set_ro(fd, NULL /* fname? */); + if (n != 0) { + (void) fprintf(stderr, "Set R/O, err=%d\n", n); + exit(1); + } + + /* + * This fails, but write on an already open handle should succeed + * the same as when we've set the mode to 444 after open. + */ + if (vflag) + (void) fprintf(stderr, "Write 3 (DOS R/O)\n"); + n = write(fd, "Write DOS RO?\n", 14); + if (n != 14) { + (void) fprintf(stderr, "write (DOS R/O), err=%d\n", errno); + exit(1); + } + + return (0); +} diff --git a/usr/src/test/zfs-tests/include/commands.cfg b/usr/src/test/zfs-tests/include/commands.cfg index afc3153e6ecf..462f0e55e3fa 100644 --- a/usr/src/test/zfs-tests/include/commands.cfg +++ b/usr/src/test/zfs-tests/include/commands.cfg @@ -172,6 +172,7 @@ export SBIN_FILES='fdisk export ZFSTEST_FILES='chg_usr_exec devname2devid dir_rd_update + dos_ro file_check file_trunc file_write diff --git a/usr/src/test/zfs-tests/runfiles/delphix.run b/usr/src/test/zfs-tests/runfiles/delphix.run index df1b6369b3f1..aaeb65de38d9 100644 --- a/usr/src/test/zfs-tests/runfiles/delphix.run +++ b/usr/src/test/zfs-tests/runfiles/delphix.run @@ -25,7 +25,8 @@ post = cleanup outputdir = /var/tmp/test_results [/opt/zfs-tests/tests/functional/acl/cifs] -tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos'] +tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos', + 'cifs_attr_004_pos'] [/opt/zfs-tests/tests/functional/acl/nontrivial] tests = ['zfs_acl_chmod_001_neg', 'zfs_acl_chmod_002_pos', diff --git a/usr/src/test/zfs-tests/runfiles/omnios.run b/usr/src/test/zfs-tests/runfiles/omnios.run index 952ab1503dd9..df7dbb2d0ee3 100644 --- a/usr/src/test/zfs-tests/runfiles/omnios.run +++ b/usr/src/test/zfs-tests/runfiles/omnios.run @@ -25,7 +25,8 @@ post = cleanup outputdir = /var/tmp/test_results [/opt/zfs-tests/tests/functional/acl/cifs] -tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos'] +tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos', + 'cifs_attr_004_pos'] [/opt/zfs-tests/tests/functional/acl/nontrivial] tests = ['zfs_acl_chmod_001_neg', 'zfs_acl_chmod_002_pos', diff --git a/usr/src/test/zfs-tests/runfiles/openindiana.run b/usr/src/test/zfs-tests/runfiles/openindiana.run index 407af74e5705..81672aac694d 100644 --- a/usr/src/test/zfs-tests/runfiles/openindiana.run +++ b/usr/src/test/zfs-tests/runfiles/openindiana.run @@ -25,7 +25,8 @@ post = cleanup outputdir = /var/tmp/test_results [/opt/zfs-tests/tests/functional/acl/cifs] -tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos'] +tests = ['cifs_attr_001_pos', 'cifs_attr_002_pos', 'cifs_attr_003_pos', + 'cifs_attr_004_pos'] [/opt/zfs-tests/tests/functional/acl/nontrivial] tests = ['zfs_acl_chmod_001_neg', 'zfs_acl_chmod_002_pos', diff --git a/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_003_pos.ksh b/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_003_pos.ksh index a47ad0277bf8..09ddc4281a4d 100644 --- a/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_003_pos.ksh +++ b/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_003_pos.ksh @@ -376,6 +376,7 @@ function unit_writeacl function test_readonly { typeset object=$1 + typeset exp if [[ -z $object ]]; then log_fail "Object($object) not defined." @@ -394,14 +395,21 @@ function test_readonly log_must set_attribute $object "R" - unit_writefile $object $user 1 + # As with mode bits, root can bypass. + if [[ "$user" == "root" ]]; then + exp=0 + else + exp=1 + fi + + unit_writefile $object $user $exp unit_writedir $object $user - unit_appenddata $object $user 1 + unit_appenddata $object $user $exp if [[ -d $object ]]; then unit_writexattr $object $user else - unit_writexattr $object $user 1 + unit_writexattr $object $user $exp fi unit_accesstime $object $user diff --git a/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_004_pos.ksh b/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_004_pos.ksh new file mode 100644 index 000000000000..b5055544b999 --- /dev/null +++ b/usr/src/test/zfs-tests/tests/functional/acl/cifs/cifs_attr_004_pos.ksh @@ -0,0 +1,141 @@ +#!/bin/ksh -p +# +# CDDL HEADER START +# +# The contents of this file are subject to the terms of the +# Common Development and Distribution License (the "License"). +# You may not use this file except in compliance with the License. +# +# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE +# or http://www.opensolaris.org/os/licensing. +# See the License for the specific language governing permissions +# and limitations under the License. +# +# When distributing Covered Code, include this CDDL HEADER in each +# file and include the License file at usr/src/OPENSOLARIS.LICENSE. +# If applicable, add the following below this CDDL HEADER, with the +# fields enclosed by brackets "[]" replaced with your own identifying +# information: Portions Copyright [yyyy] [name of copyright owner] +# +# CDDL HEADER END +# + +# +# Copyright 2009 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +# +# Copyright (c) 2012 by Delphix. All rights reserved. +# Copyright 2017 Nexenta Systems, Inc. All rights reserved. +# + +. $STF_SUITE/tests/functional/acl/acl_common.kshlib +. $STF_SUITE/tests/functional/acl/cifs/cifs.kshlib + +# +# DESCRIPTION: +# Verify the ability to continue writing to a file +# after opening the file read/write, and setting +# the DOS Readonly flag on that file. +# +# STRATEGY: +# Run the special program "dos_ro" + +verify_runnable "both" + +function cleanup +{ + if [[ -n $gobject ]]; then + destroy_object $gobject + fi + + for fs in $TESTPOOL/$TESTFS $TESTPOOL ; do + mtpt=$(get_prop mountpoint $fs) + log_must rm -rf $mtpt/file.* $mtpt/dir.* + done + + [[ -f $TESTFILE ]] && rm $TESTFILE +} + +# +# Set the special attribute to the given node +# +# $1: The given node (file/dir) +# $2: The special attribute to be set +# +function set_attribute +{ + typeset object=$1 + typeset attr=$2 + + if [[ -z $attr ]]; then + attr="AHRSadimu" + if [[ -f $object ]]; then + attr="${attr}q" + fi + fi + chmod S+c${attr} $object + return $? +} + +# +# Clear the special attribute to the given node +# +# $1: The given node (file/dir) +# $2: The special attribute to be cleared +# +function clear_attribute +{ + typeset object=$1 + typeset attr=$2 + + if [[ -z $attr ]]; then + if is_global_zone ; then + attr="AHRSadimu" + if [[ -f $object ]]; then + attr="${attr}q" + fi + else + attr="AHRS" + fi + fi + + chmod S-c${attr} $object + return $? +} + +FILES="file.0 file.1" +FS="$TESTPOOL $TESTPOOL/$TESTFS" +ATTRS="R" + +TESTFILE=/tmp/tfile +TESTDIR=tdir +TESTATTR=tattr +TESTACL=user:$ZFS_ACL_OTHER1:write_data:allow +TESTMODE=777 +TESTSTR="ZFS test suites" + +log_assert "Verify writable open handle still works after " \ + "setting the DOS Readonly flag on a file." +log_onexit cleanup + +echo "$TESTSTR" > $TESTFILE + +typeset gobject +typeset gattr +for fs in $FS ; do + mtpt=$(get_prop mountpoint $fs) + chmod 777 $mtpt + for user in root $ZFS_ACL_STAFF1; do + log_must set_cur_usr $user + for file in $FILES ; do + gobject=$mtpt/$file + create_object "file" $gobject $ZFS_ACL_CUR_USER + log_must dos_ro $gobject + destroy_object $gobject + done + done +done + +log_pass "Writable handle OK after setting DOS R/O flag." diff --git a/usr/src/uts/common/fs/zfs/zfs_acl.c b/usr/src/uts/common/fs/zfs/zfs_acl.c index f2ef7e513435..519b4355d5dc 100644 --- a/usr/src/uts/common/fs/zfs/zfs_acl.c +++ b/usr/src/uts/common/fs/zfs/zfs_acl.c @@ -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 @@ -2035,13 +2035,11 @@ zfs_zaccess_dataset_check(znode_t *zp, uint32_t v4_mode) } /* - * 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)); } @@ -2253,6 +2251,24 @@ zfs_zaccess_common(znode_t *zp, uint32_t v4_mode, uint32_t *working_mode, 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)); } diff --git a/usr/src/uts/common/fs/zfs/zfs_vnops.c b/usr/src/uts/common/fs/zfs/zfs_vnops.c index 250ed05f1def..b2a5cf51b5a1 100644 --- a/usr/src/uts/common/fs/zfs/zfs_vnops.c +++ b/usr/src/uts/common/fs/zfs/zfs_vnops.c @@ -707,9 +707,11 @@ zfs_write(vnode_t *vp, uio_t *uio, int ioflag, cred_t *cr, caller_context_t *ct) } /* - * 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_t *cr, 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, caddr_t *addrp, 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)); }