From 5e7198b87353c478b3f4a18c7b48a8f37dac15c6 Mon Sep 17 00:00:00 2001 From: Ryan Moeller Date: Fri, 9 Oct 2020 12:27:14 -0400 Subject: [PATCH] Linux: Initialize zp in zfs_setattr_dir The value of zp is used without having been initialized under some conditions. Initialize the pointer to NULL. Add a regression test case using chown in acl/posix. However, this is not enough because the setup sets xattr=sa, which means zfs_setattr_dir will not be called. Create a second group of acl tests in acl/posix-sa duplicating the acl/posix tests with symlinks, and remove xattr=sa from the original acl/posix tests. This provides more coverage for the default xattr=on code. Reviewed-by: Brian Behlendorf Signed-off-by: Ryan Moeller Closes #10043 Closes #11025 --- configure.ac | 1 + man/man8/zfsprops.8 | 2 +- module/os/linux/zfs/zfs_vnops.c | 2 +- tests/runfiles/linux.run | 6 ++- .../tests/functional/acl/Makefile.am | 2 +- .../tests/functional/acl/posix-sa/Makefile.am | 8 +++ .../tests/functional/acl/posix-sa/cleanup.ksh | 33 ++++++++++++ .../functional/acl/posix-sa/posix_001_pos.ksh | 1 + .../functional/acl/posix-sa/posix_002_pos.ksh | 1 + .../functional/acl/posix-sa/posix_003_pos.ksh | 1 + .../functional/acl/posix-sa/posix_004_pos.ksh | 1 + .../tests/functional/acl/posix-sa/setup.ksh | 52 +++++++++++++++++++ .../tests/functional/acl/posix/Makefile.am | 3 +- .../functional/acl/posix/posix_004_pos.ksh | 49 +++++++++++++++++ .../tests/functional/acl/posix/setup.ksh | 1 - 15 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 tests/zfs-tests/tests/functional/acl/posix-sa/Makefile.am create mode 100755 tests/zfs-tests/tests/functional/acl/posix-sa/cleanup.ksh create mode 120000 tests/zfs-tests/tests/functional/acl/posix-sa/posix_001_pos.ksh create mode 120000 tests/zfs-tests/tests/functional/acl/posix-sa/posix_002_pos.ksh create mode 120000 tests/zfs-tests/tests/functional/acl/posix-sa/posix_003_pos.ksh create mode 120000 tests/zfs-tests/tests/functional/acl/posix-sa/posix_004_pos.ksh create mode 100755 tests/zfs-tests/tests/functional/acl/posix-sa/setup.ksh create mode 100755 tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh diff --git a/configure.ac b/configure.ac index a1664151bc..9323aa7a0c 100644 --- a/configure.ac +++ b/configure.ac @@ -237,6 +237,7 @@ AC_CONFIG_FILES([ tests/zfs-tests/tests/functional/Makefile tests/zfs-tests/tests/functional/acl/Makefile tests/zfs-tests/tests/functional/acl/posix/Makefile + tests/zfs-tests/tests/functional/acl/posix-sa/Makefile tests/zfs-tests/tests/functional/alloc_class/Makefile tests/zfs-tests/tests/functional/arc/Makefile tests/zfs-tests/tests/functional/atime/Makefile diff --git a/man/man8/zfsprops.8 b/man/man8/zfsprops.8 index 2c4a2af294..a3392d6c00 100644 --- a/man/man8/zfsprops.8 +++ b/man/man8/zfsprops.8 @@ -1794,7 +1794,7 @@ on platforms which do not support the feature. .Pp The use of system attribute based xattrs is strongly encouraged for users of -SELinux or POSIX ACLs. Both of these features heavily rely of extended +SELinux or POSIX ACLs. Both of these features heavily rely on extended attributes and benefit significantly from the reduced access time. .Pp The values diff --git a/module/os/linux/zfs/zfs_vnops.c b/module/os/linux/zfs/zfs_vnops.c index 0932b9b330..b668c7dff0 100644 --- a/module/os/linux/zfs/zfs_vnops.c +++ b/module/os/linux/zfs/zfs_vnops.c @@ -2543,7 +2543,7 @@ zfs_setattr_dir(znode_t *dzp) zap_cursor_t zc; zap_attribute_t zap; zfs_dirlock_t *dl; - znode_t *zp; + znode_t *zp = NULL; dmu_tx_t *tx = NULL; uint64_t uid, gid; sa_bulk_attr_t bulk[4]; diff --git a/tests/runfiles/linux.run b/tests/runfiles/linux.run index 94964434e9..ac4d6af1cf 100644 --- a/tests/runfiles/linux.run +++ b/tests/runfiles/linux.run @@ -23,9 +23,13 @@ outputdir = /var/tmp/test_results tags = ['functional'] [tests/functional/acl/posix:Linux] -tests = ['posix_001_pos', 'posix_002_pos', 'posix_003_pos'] +tests = ['posix_001_pos', 'posix_002_pos', 'posix_003_pos', 'posix_004_pos'] tags = ['functional', 'acl', 'posix'] +[tests/functional/acl/posix-sa:Linux] +tests = ['posix_001_pos', 'posix_002_pos', 'posix_003_pos', 'posix_004_pos'] +tags = ['functional', 'acl', 'posix-sa'] + [tests/functional/atime:Linux] tests = ['atime_003_pos', 'root_relatime_on'] tags = ['functional', 'atime'] diff --git a/tests/zfs-tests/tests/functional/acl/Makefile.am b/tests/zfs-tests/tests/functional/acl/Makefile.am index 6086930e36..382bb5f064 100644 --- a/tests/zfs-tests/tests/functional/acl/Makefile.am +++ b/tests/zfs-tests/tests/functional/acl/Makefile.am @@ -3,4 +3,4 @@ dist_pkgdata_DATA = \ acl.cfg \ acl_common.kshlib -SUBDIRS = posix +SUBDIRS = posix posix-sa diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/Makefile.am b/tests/zfs-tests/tests/functional/acl/posix-sa/Makefile.am new file mode 100644 index 0000000000..31d1237ce2 --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/Makefile.am @@ -0,0 +1,8 @@ +pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/acl/posix-sa +dist_pkgdata_SCRIPTS = \ + cleanup.ksh \ + setup.ksh \ + posix_001_pos.ksh \ + posix_002_pos.ksh \ + posix_003_pos.ksh \ + posix_004_pos.ksh diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/cleanup.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/cleanup.ksh new file mode 100755 index 0000000000..bb58a8cf2e --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/cleanup.ksh @@ -0,0 +1,33 @@ +#!/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 2007 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/acl/acl_common.kshlib + +cleanup_user_group + +default_cleanup diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/posix_001_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_001_pos.ksh new file mode 120000 index 0000000000..e6467b3470 --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_001_pos.ksh @@ -0,0 +1 @@ +../posix/posix_001_pos.ksh \ No newline at end of file diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/posix_002_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_002_pos.ksh new file mode 120000 index 0000000000..10140d0e87 --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_002_pos.ksh @@ -0,0 +1 @@ +../posix/posix_002_pos.ksh \ No newline at end of file diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/posix_003_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_003_pos.ksh new file mode 120000 index 0000000000..3f3db2807d --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_003_pos.ksh @@ -0,0 +1 @@ +../posix/posix_003_pos.ksh \ No newline at end of file diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/posix_004_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_004_pos.ksh new file mode 120000 index 0000000000..2c2bab4477 --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/posix_004_pos.ksh @@ -0,0 +1 @@ +../posix/posix_004_pos.ksh \ No newline at end of file diff --git a/tests/zfs-tests/tests/functional/acl/posix-sa/setup.ksh b/tests/zfs-tests/tests/functional/acl/posix-sa/setup.ksh new file mode 100755 index 0000000000..d8bf8a638e --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix-sa/setup.ksh @@ -0,0 +1,52 @@ +#!/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 2007 Sun Microsystems, Inc. All rights reserved. +# Use is subject to license terms. +# + +# +# Copyright (c) 2016 by Delphix. All rights reserved. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/acl/acl_common.kshlib + +log_must getfacl --version +log_must setfacl --version + +cleanup_user_group + +# Create staff group and add user to it +log_must add_group $ZFS_ACL_STAFF_GROUP +log_must add_user $ZFS_ACL_STAFF_GROUP $ZFS_ACL_STAFF1 + +DISK=${DISKS%% *} +default_setup_noexit $DISK +log_must chmod 777 $TESTDIR + +# Use POSIX ACLs on filesystem +log_must zfs set acltype=posix $TESTPOOL/$TESTFS +log_must zfs set xattr=sa $TESTPOOL/$TESTFS + +log_pass diff --git a/tests/zfs-tests/tests/functional/acl/posix/Makefile.am b/tests/zfs-tests/tests/functional/acl/posix/Makefile.am index dcf2788580..e63f63185a 100644 --- a/tests/zfs-tests/tests/functional/acl/posix/Makefile.am +++ b/tests/zfs-tests/tests/functional/acl/posix/Makefile.am @@ -4,4 +4,5 @@ dist_pkgdata_SCRIPTS = \ setup.ksh \ posix_001_pos.ksh \ posix_002_pos.ksh \ - posix_003_pos.ksh + posix_003_pos.ksh \ + posix_004_pos.ksh diff --git a/tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh b/tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh new file mode 100755 index 0000000000..6c6b592fbb --- /dev/null +++ b/tests/zfs-tests/tests/functional/acl/posix/posix_004_pos.ksh @@ -0,0 +1,49 @@ +#!/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 +# + +# +# Portions Copyright 2020 iXsystems, Inc. +# + +. $STF_SUITE/include/libtest.shlib +. $STF_SUITE/tests/functional/acl/acl_common.kshlib + +# +# DESCRIPTION: +# Verify chown works with POSIX ACLs. +# Regression test for https://github.com/openzfs/zfs/issues/10043 +# +# STRATEGY: +# 1. Prepare an appropriate ACL on the test directory +# 2. Change the owner of the directory +# + +verify_runnable "both" +log_assert "Verify chown works with POSIX ACLs" + +log_must setfacl -d -m u:$ZFS_ACL_STAFF1:rwx $TESTDIR +log_must setfacl -b $TESTDIR + +log_must chown $ZFS_ACL_STAFF1 $TESTDIR +log_must chown 0 $TESTDIR + +log_pass "chown works with POSIX ACLs" diff --git a/tests/zfs-tests/tests/functional/acl/posix/setup.ksh b/tests/zfs-tests/tests/functional/acl/posix/setup.ksh index d8bf8a638e..526c78e17f 100755 --- a/tests/zfs-tests/tests/functional/acl/posix/setup.ksh +++ b/tests/zfs-tests/tests/functional/acl/posix/setup.ksh @@ -47,6 +47,5 @@ log_must chmod 777 $TESTDIR # Use POSIX ACLs on filesystem log_must zfs set acltype=posix $TESTPOOL/$TESTFS -log_must zfs set xattr=sa $TESTPOOL/$TESTFS log_pass