* [PATCH] btrfs: should add a permission check for setfacl
@ 2010-05-18 0:50 Shi Weihua
2010-05-20 8:33 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Shi Weihua @ 2010-05-18 0:50 UTC (permalink / raw)
To: chris.mason, Yan, Zheng; +Cc: linux-btrfs, LKML
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 <shiwh@cn.fujitsu.com>
---
diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
index da3133c..12d7be8 100644
--- a/fs/btrfs/acl.c
+++ b/fs/btrfs/acl.c
@@ -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) {
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: should add a permission check for setfacl
2010-05-18 0:50 [PATCH] btrfs: should add a permission check for setfacl Shi Weihua
@ 2010-05-20 8:33 ` Christoph Hellwig
2010-05-24 6:38 ` Shi Weihua
0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2010-05-20 8:33 UTC (permalink / raw)
To: Shi Weihua; +Cc: chris.mason, Yan, Zheng, linux-btrfs, LKML
On Tue, May 18, 2010 at 08:50:32AM +0800, Shi Weihua wrote:
> 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
Can you add this as a new testcase to xfstests so that we can easiy
check for regressions and future filesystems implementing this
correctly?
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: should add a permission check for setfacl
2010-05-20 8:33 ` Christoph Hellwig
@ 2010-05-24 6:38 ` Shi Weihua
2010-05-27 19:19 ` Christoph Hellwig
0 siblings, 1 reply; 4+ messages in thread
From: Shi Weihua @ 2010-05-24 6:38 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: chris.mason, Yan, Zheng, linux-btrfs, xfs
cc xfstests ml
at 2010-5-20 16:33, Christoph Hellwig wrote:
> On Tue, May 18, 2010 at 08:50:32AM +0800, Shi Weihua wrote:
>> 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
>
> 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 <shiwh@cn.fujitsu.com>
---
diff -urpN xfstests.orig.229/230 xfstests/230
--- xfstests.orig.229/230 1970-01-01 08:00:00.000000000 +0800
+++ xfstests/230 2010-05-28 14:27:02.000000000 +0800
@@ -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
+owner=shiwh@cn.fujitsu.com
+
+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
diff -urpN xfstests.orig.229/230.out xfstests/230.out
--- xfstests.orig.229/230.out 1970-01-01 08:00:00.000000000 +0800
+++ xfstests/230.out 2010-05-28 14:27:05.000000000 +0800
@@ -0,0 +1,4 @@
+QA output created by 230
+Expect to FAIL
+setfacl: file1: Operation not permitted
+Test over.
diff -urpN xfstests.orig.229/group xfstests/group
--- xfstests.orig.229/group 2010-05-28 11:29:31.000000000 +0800
+++ xfstests/group 2010-05-28 14:26:48.000000000 +0800
@@ -343,3 +343,4 @@ deprecated
227 auto fsr
228 rw auto prealloc quick
229 auto
+230 acl auto
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: should add a permission check for setfacl
2010-05-24 6:38 ` Shi Weihua
@ 2010-05-27 19:19 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2010-05-27 19:19 UTC (permalink / raw)
To: Shi Weihua; +Cc: Christoph Hellwig, Yan, Zheng, linux-btrfs, chris.mason, xfs
Thanks! I've commit both patches.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-27 19:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-18 0:50 [PATCH] btrfs: should add a permission check for setfacl Shi Weihua
2010-05-20 8:33 ` Christoph Hellwig
2010-05-24 6:38 ` Shi Weihua
2010-05-27 19:19 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).