Skip to contentSkip to conversation

btrfs: should add a permission check for setfacl

PATCH In review Shi Weihua 2 participants 4 messages May 18, 2010 – May 27, 2010 Btrfs file system

lore
Share & tools
lore

Shi Weihua

author
#0 lore raw

PATCH 1/1 btrfs: should add a permission check for setfacl

On btrfs, do the following
------------------

# su user1
# cd btrfs-part/
# touch aaa
# getfacl aaa
  # file: aaa
  # owner: user1
  # group: user1
  user::rw-
  group::rw-
  other::r--
# su user2
# cd btrfs-part/
# setfacl -m u::rwx aaa
# getfacl aaa
  # file: aaa
  # owner: user1
  # group: user1
  user::rwx           <- successed to setfacl
  group::rw-
  other::r--
------------------
but we should prohibit it that user2 changing user1's acl.
In fact, on ext3 and other fs, a message occurs:
  setfacl: aaa: Operation not permitted

This patch fixed it.

Signed-off-by Shi Weihua <[email protected]>

---

fs/btrfs/acl.c +3 -0
index da3133c..12d7be8 100644
@@ -159,6 +159,9 @@ static int btrfs_xattr_set_acl(struct inode *inode, int type,
 	int ret;
 	struct posix_acl *acl = NULL;
 
+	if (!is_owner_or_cap(inode))
+		return -EPERM;
+
 	if (value) {
 		acl = posix_acl_from_xattr(value, size);
 		if (acl == NULL) {
#1 lore raw
Quoted 18 lines · Shi Weihua

On btrfs, do the following
------------------

# su user1
# cd btrfs-part/
# touch aaa
# getfacl aaa
  # file: aaa
  # owner: user1
  # group: user1
  user::rw-
  group::rw-
  other::r--
# su user2
# cd btrfs-part/
# setfacl -m u::rwx aaa
# getfacl aaa
  # file: aaa
  # owner: user1
Quoted 8 lines
  # group: user1
  user::rwx           <- successed to setfacl
  group::rw-
  other::r--
------------------
but we should prohibit it that user2 changing user1's acl.
In fact, on ext3 and other fs, a message occurs:
  setfacl: aaa: Operation not permitted

Can you add this as a new testcase to xfstests so that we can easiy check for regressions and future filesystems implementing this correctly?

author
#2 lore raw

cc xfstests ml

Quoted 28 lines · Christoph Hellwig
Quoted 18 lines · Shi Weihua

On btrfs, do the following
------------------

# su user1
# cd btrfs-part/
# touch aaa
# getfacl aaa
  # file: aaa
  # owner: user1
  # group: user1
  user::rw-
  group::rw-
  other::r--
# su user2
# cd btrfs-part/
# setfacl -m u::rwx aaa
# getfacl aaa
  # file: aaa
  # owner: user1
Quoted 8 lines
  # group: user1
  user::rwx           <- successed to setfacl
  group::rw-
  other::r--
------------------
but we should prohibit it that user2 changing user1's acl.
In fact, on ext3 and other fs, a message occurs:
  setfacl: aaa: Operation not permitted
Quoted 4 lines

Can you add this as a new testcase to xfstests so that we can easiy check for regressions and future filesystems implementing this correctly?

did it. maybe it should be merged into 051 or 099.

Signed-off-by Shi Weihua <[email protected]>

---

230 +80 -0
diff -urpN xfstests.orig.229/230 xfstests/230
@@ -0,0 +1,80 @@
+#! /bin/bash
+# FS QA Test No. 230
+#
+# Check user B can setfacl a file which belongs to user A
+# See also http://marc.info/?l=linux-btrfs&m=127434445620298&w=2
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2010 FUJITSU LIMITED. All Rights Reserved.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#
+#-----------------------------------------------------------------------
+#
+# creator
[email protected]
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+runas=$here/src/runas
+status=1        # FAILure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.attr
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+	[ -n "$testdir" ] && rm -rf $testdir/$seq.dir1
+	_cleanup_testdir
+}
+
+# real QA test starts here
+_supported_fs generic
+# only Linux supports fallocate
+_supported_os Linux
+
+[ -x $runas ] || _notrun "$runas executable not found"
+
+rm -f $seq.full
+
+_setup_testdir
+
+_need_to_be_root
+_acl_setup_ids
+_require_acls
+
+# get dir
+cd $testdir
+rm -rf $seq.dir1
+mkdir $seq.dir1
+cd $seq.dir1
+
+touch file1
+chown $acl1.$acl1 file1
+
+echo "Expect to FAIL"
+$runas -u $acl2 -g $acl2 -- `which setfacl` -m u::rwx file1 2>&1
+
+echo "Test over."
+# success, all done
+status=0
+exit
230.out +4 -0
diff -urpN xfstests.orig.229/230.out xfstests/230.out
@@ -0,0 +1,4 @@
+QA output created by 230
+Expect to FAIL
+setfacl: file1: Operation not permitted
+Test over.
group +1 -0
diff -urpN xfstests.orig.229/group xfstests/group
@@ -343,3 +343,4 @@ deprecated
 227 auto fsr
 228 rw auto prealloc quick
 229 auto
+230 acl auto

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs