Skip to content

Commit a340199

Browse files
anodos325Ryan Moeller
anodos325
authored and
Ryan Moeller
committed
Fix regression in POSIX mode behavior
Commit 235a856 introduced a regression in evaluation of POSIX modes that require group DENY entries in the internal ZFS ACL. An example of such a POSX mode is 007. When write_implies_delete_child is set, then ACE_WRITE_DATA is added to `wanted_dirperms` in prior to calling zfs_zaccess_common(). This occurs is zfs_zaccess_delete(). Unfortunately, when zfs_zaccess_aces_check hits this particular DENY ACE, zfs_groupmember() is checked to determine whether access should be denied, and since zfs_groupmember() always returns B_TRUE on Linux and so this check is failed, resulting ultimately in EPERM being returned. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ryan Moeller <ryan@iXsystems.com> Signed-off-by: Andrew Walker <awalker@ixsystems.com> Closes openzfs#11760
1 parent 418add4 commit a340199

File tree

5 files changed

+154
-4
lines changed

5 files changed

+154
-4
lines changed

‎module/zfs/zfs_fuid.c

-4
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,6 @@ zfs_fuid_info_free(zfs_fuid_info_t *fuidp)
728728
boolean_t
729729
zfs_groupmember(zfsvfs_t *zfsvfs, uint64_t id, cred_t *cr)
730730
{
731-
#ifdef HAVE_KSID
732731
uid_t gid;
733732

734733
#ifdef illumos
@@ -773,9 +772,6 @@ zfs_groupmember(zfsvfs_t *zfsvfs, uint64_t id, cred_t *cr)
773772
*/
774773
gid = zfs_fuid_map_id(zfsvfs, id, cr, ZFS_GROUP);
775774
return (groupmember(gid, cr));
776-
#else
777-
return (B_TRUE);
778-
#endif
779775
}
780776

781777
void

‎tests/runfiles/common.run

+4
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ failsafe = callbacks/zfs_failsafe
2828
outputdir = /var/tmp/test_results
2929
tags = ['functional']
3030

31+
[tests/functional/acl/off]
32+
tests = ['posixmode']
33+
tags = ['functional', 'acl']
34+
3135
[tests/functional/alloc_class]
3236
tests = ['alloc_class_001_pos', 'alloc_class_002_neg', 'alloc_class_003_pos',
3337
'alloc_class_004_pos', 'alloc_class_005_pos', 'alloc_class_006_pos',

‎tests/runfiles/sanity.run

+4
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@ failsafe = callbacks/zfs_failsafe
3030
outputdir = /var/tmp/test_results
3131
tags = ['functional']
3232

33+
[tests/functional/acl/off]
34+
tests = ['posixmode']
35+
tags = ['functional', 'acl']
36+
3337
[tests/functional/alloc_class]
3438
tests = ['alloc_class_003_pos', 'alloc_class_004_pos', 'alloc_class_005_pos',
3539
'alloc_class_006_pos', 'alloc_class_008_pos', 'alloc_class_010_pos',

‎tests/zfs-tests/tests/functional/acl/off/Makefile.am

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ pkgdatadir = $(datadir)/@PACKAGE@/zfs-tests/tests/functional/acl/off
44

55
dist_pkgdata_SCRIPTS = \
66
dosmode.ksh \
7+
posixmode.ksh \
78
cleanup.ksh \
89
setup.ksh
910

‎tests/zfs-tests/tests/functional/acl/off/posixmode.ksh

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
#!/bin/ksh -p
2+
#
3+
# CDDL HEADER START
4+
#
5+
# The contents of this file are subject to the terms of the
6+
# Common Development and Distribution License (the "License").
7+
# You may not use this file except in compliance with the License.
8+
#
9+
# You can obtain a copy of the license at usr/src/OPENSOLARIS.LICENSE
10+
# or http://www.opensolaris.org/os/licensing.
11+
# See the License for the specific language governing permissions
12+
# and limitations under the License.
13+
#
14+
# When distributing Covered Code, include this CDDL HEADER in each
15+
# file and include the License file at usr/src/OPENSOLARIS.LICENSE.
16+
# If applicable, add the following below this CDDL HEADER, with the
17+
# fields enclosed by brackets "[]" replaced with your own identifying
18+
# information: Portions Copyright [yyyy] [name of copyright owner]
19+
#
20+
# CDDL HEADER END
21+
#
22+
23+
#
24+
# Portions Copyright 2021 iXsystems, Inc.
25+
#
26+
27+
. $STF_SUITE/include/libtest.shlib
28+
. $STF_SUITE/tests/functional/acl/acl_common.kshlib
29+
30+
#
31+
# DESCRIPTION:
32+
# Verify that POSIX mode bits function correctly.
33+
#
34+
# These tests are incomplete and will be added to over time.
35+
#
36+
# NOTE: Creating directory entries behaves differently between platforms.
37+
# The parent directory's group is used on FreeBSD, while the effective
38+
# group is used on Linux. We chown to the effective group when creating
39+
# directories and files in these tests to achieve consistency across all
40+
# platforms.
41+
#
42+
# STRATEGY:
43+
# 1. Sanity check the POSIX mode test on tmpfs
44+
# 2. Test POSIX mode bits on ZFS
45+
#
46+
47+
verify_runnable "both"
48+
49+
function cleanup
50+
{
51+
umount -f $tmpdir
52+
rm -rf $tmpdir $TESTDIR/dir
53+
}
54+
55+
log_assert "Verify POSIX mode bits function correctly"
56+
log_onexit cleanup
57+
58+
owner=$ZFS_ACL_STAFF1
59+
other=$ZFS_ACL_STAFF2
60+
group=$ZFS_ACL_STAFF_GROUP
61+
if is_linux; then
62+
wheel=root
63+
else
64+
wheel=wheel
65+
fi
66+
67+
function test_posix_mode # base
68+
{
69+
typeset base=$1
70+
typeset dir=$base/dir
71+
typeset file=$dir/file
72+
73+
# dir owned by root
74+
log_must mkdir $dir
75+
log_must chown :$wheel $dir
76+
log_must chmod 007 $dir
77+
78+
# file owned by root
79+
log_must touch $file
80+
log_must chown :$wheel $file
81+
log_must ls -la $dir
82+
log_must rm $file
83+
84+
log_must touch $file
85+
log_must chown :$wheel $file
86+
log_must user_run $other rm $file
87+
88+
# file owned by user
89+
log_must user_run $owner touch $file
90+
log_must chown :$group $file
91+
log_must ls -la $dir
92+
log_must user_run $owner rm $file
93+
94+
log_must user_run $owner touch $file
95+
log_must chown :$group $file
96+
log_must user_run $other rm $file
97+
98+
log_must user_run $owner touch $file
99+
log_must chown :$group $file
100+
log_must rm $file
101+
102+
log_must rm -rf $dir
103+
104+
# dir owned by user
105+
log_must user_run $owner mkdir $dir
106+
log_must chown :$group $dir
107+
log_must user_run $owner chmod 007 $dir
108+
109+
# file owned by root
110+
log_must touch $file
111+
log_must chown :$wheel $file
112+
log_must ls -la $dir
113+
log_must rm $file
114+
115+
log_must touch $file
116+
log_must chown :$wheel $file
117+
log_mustnot user_run $other rm $file
118+
log_must rm $file
119+
120+
# file owned by user
121+
log_mustnot user_run $owner touch $file
122+
log_must touch $file
123+
log_must chown $owner:$group $file
124+
log_must ls -la $dir
125+
log_mustnot user_run $owner rm $file
126+
log_mustnot user_run $other rm $file
127+
log_must rm $file
128+
129+
log_must rm -rf $dir
130+
}
131+
132+
# Sanity check on tmpfs first
133+
tmpdir=$(TMPDIR=$TEST_BASE_DIR mktemp -d)
134+
log_must mount -t tmpfs tmp $tmpdir
135+
log_must chmod 777 $tmpdir
136+
137+
test_posix_mode $tmpdir
138+
139+
log_must umount $tmpdir
140+
log_must rmdir $tmpdir
141+
142+
# Verify ZFS
143+
test_posix_mode $TESTDIR
144+
145+
log_pass "POSIX mode bits function correctly"

0 commit comments

Comments
 (0)