All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xfstests: generic/313, test sgid inheritance on subdirs
@ 2013-05-29 22:30 Eric Sandeen
  2013-05-30 20:02 ` [PATCH V2] " Eric Sandeen
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2013-05-29 22:30 UTC (permalink / raw)
  To: xfs-oss

On XFS this test fails today due to some over-eager
SGID removal in xfs core code; ext4 & btrfs pass.
Document it in a test, will send xfs patch in a bit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s.  I'm going to just flaunt my ignorance and say:
I've never understood acls well.  I'm hoping someone who does
can help me write a better changelog, because this one
is no good at all!

diff --git a/tests/generic/313 b/tests/generic/313
new file mode 100755
index 0000000..289fd6f
--- /dev/null
+++ b/tests/generic/313
@@ -0,0 +1,67 @@
+#! /bin/bash
+# FS QA Test No. 313
+#
+# Test SGID inheritance on subdirectories
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Red Hat, Inc.  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
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs generic
+_require_acls
+_require_user
+_need_to_be_root
+
+rm -rf $TEST_DIR/$seq-dir
+
+# Make dir owned by qa user, and an unrelated group:
+mkdir $TEST_DIR/$seq-dir
+chown $qa_user:12345 $TEST_DIR/$seq-dir
+
+# Make parent dir sgid
+chmod 2775 $TEST_DIR/$seq-dir
+
+# Make subdirs before & after acl set
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir"
+su $qa_user -c "setfacl -m u:$qa_user:rwx,d:u:$qa_user:rwx $TEST_DIR/$seq-dir"
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir2"
+
+# Both subdirs should have inherited sgid
+ls -ld $TEST_DIR/$seq-dir/subdir* | _filter_test_dir | awk '{print $1,$NF}'
+
+status=0
+exit
diff --git a/tests/generic/313.out b/tests/generic/313.out
new file mode 100644
index 0000000..326a929
--- /dev/null
+++ b/tests/generic/313.out
@@ -0,0 +1,3 @@
+QA output created by 313
+drwxr-sr-x. TEST_DIR/313-dir/subdir
+drwxrwsr-x+ TEST_DIR/313-dir/subdir2
diff --git a/tests/generic/group b/tests/generic/group
index bd443c1..d5ec8d5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -115,3 +115,4 @@
 310 auto
 311 auto metadata log
 312 auto quick prealloc enospc
+313 auto quick acl

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-05-29 22:30 [PATCH] xfstests: generic/313, test sgid inheritance on subdirs Eric Sandeen
@ 2013-05-30 20:02 ` Eric Sandeen
  2013-06-12 19:23   ` Carlos Maiolino
  2013-07-12 18:51   ` [PATCH V3] xfstests: generic/314, " Eric Sandeen
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Sandeen @ 2013-05-30 20:02 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs-oss

On XFS this test fails today due to some over-eager
SGID removal in xfs core code; ext4 & btrfs pass.
Document it in a test, will send xfs patch in a bit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s.  I'm going to just flaunt my ignorance and say:
I've never understood acls well.  I'm hoping someone who does
can help me write a better changelog, because this one
is no good at all!

V2: add missing source of "attr" file for _require_acls

diff --git a/tests/generic/313 b/tests/generic/313
new file mode 100755
index 0000000..3dee609
--- /dev/null
+++ b/tests/generic/313
@@ -0,0 +1,68 @@
+#! /bin/bash
+# FS QA Test No. 313
+#
+# Test SGID inheritance on subdirectories
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Red Hat, Inc.  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
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# real QA test starts here
+_supported_fs generic
+_require_acls
+_require_user
+_need_to_be_root
+
+rm -rf $TEST_DIR/$seq-dir
+
+# Make dir owned by qa user, and an unrelated group:
+mkdir $TEST_DIR/$seq-dir
+chown $qa_user:12345 $TEST_DIR/$seq-dir
+
+# Make parent dir sgid
+chmod 2775 $TEST_DIR/$seq-dir
+
+# Make subdirs before & after acl set
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir"
+su $qa_user -c "setfacl -m u:$qa_user:rwx,d:u:$qa_user:rwx $TEST_DIR/$seq-dir"
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir2"
+
+# Both subdirs should have inherited sgid
+ls -ld $TEST_DIR/$seq-dir/subdir* | _filter_test_dir | awk '{print $1,$NF}'
+
+status=0
+exit
diff --git a/tests/generic/313.out b/tests/generic/313.out
new file mode 100644
index 0000000..326a929
--- /dev/null
+++ b/tests/generic/313.out
@@ -0,0 +1,3 @@
+QA output created by 313
+drwxr-sr-x. TEST_DIR/313-dir/subdir
+drwxrwsr-x+ TEST_DIR/313-dir/subdir2
diff --git a/tests/generic/group b/tests/generic/group
index bd443c1..d5ec8d5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -115,3 +115,4 @@
 310 auto
 311 auto metadata log
 312 auto quick prealloc enospc
+313 auto quick acl

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-05-30 20:02 ` [PATCH V2] " Eric Sandeen
@ 2013-06-12 19:23   ` Carlos Maiolino
  2013-07-08 21:51     ` Ben Myers
  2013-07-12 18:51   ` [PATCH V3] xfstests: generic/314, " Eric Sandeen
  1 sibling, 1 reply; 10+ messages in thread
From: Carlos Maiolino @ 2013-06-12 19:23 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Eric Sandeen, xfs-oss

I'm personally working on this bug with Eric, and the test properly trigger the
bug on 100% of the test runs, but, it should also ensure that
`irix_sgid_inherit` sysctl is set to 0, if not, the test will fail.

follow is a suggestion for a better changelog:

Tests if subdirectories created on the filesystem will properly inherit sgid bit
when this is set on the parent directory, once the process has the properly
permissions to create a subdirectory, this, should inherit parent's sgid bit if
this is set and irix_sgid_inherit sysctl is disabled.

Thanks for the test Eric, very helpful :)
On Thu, May 30, 2013 at 03:02:06PM -0500, Eric Sandeen wrote:
> On XFS this test fails today due to some over-eager
> SGID removal in xfs core code; ext4 & btrfs pass.
> Document it in a test, will send xfs patch in a bit.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> p.s.  I'm going to just flaunt my ignorance and say:
> I've never understood acls well.  I'm hoping someone who does
> can help me write a better changelog, because this one
> is no good at all!
> 
> V2: add missing source of "attr" file for _require_acls
> 
> diff --git a/tests/generic/313 b/tests/generic/313
> new file mode 100755
> index 0000000..3dee609
> --- /dev/null
> +++ b/tests/generic/313
> @@ -0,0 +1,68 @@
> +#! /bin/bash
> +# FS QA Test No. 313
> +#
> +# Test SGID inheritance on subdirectories
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2013 Red Hat, Inc.  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
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_acls
> +_require_user
> +_need_to_be_root
> +
> +rm -rf $TEST_DIR/$seq-dir
> +
> +# Make dir owned by qa user, and an unrelated group:
> +mkdir $TEST_DIR/$seq-dir
> +chown $qa_user:12345 $TEST_DIR/$seq-dir
> +
> +# Make parent dir sgid
> +chmod 2775 $TEST_DIR/$seq-dir
> +
> +# Make subdirs before & after acl set
> +su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir"
> +su $qa_user -c "setfacl -m u:$qa_user:rwx,d:u:$qa_user:rwx $TEST_DIR/$seq-dir"
> +su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir2"
> +
> +# Both subdirs should have inherited sgid
> +ls -ld $TEST_DIR/$seq-dir/subdir* | _filter_test_dir | awk '{print $1,$NF}'
> +
> +status=0
> +exit
> diff --git a/tests/generic/313.out b/tests/generic/313.out
> new file mode 100644
> index 0000000..326a929
> --- /dev/null
> +++ b/tests/generic/313.out
> @@ -0,0 +1,3 @@
> +QA output created by 313
> +drwxr-sr-x. TEST_DIR/313-dir/subdir
> +drwxrwsr-x+ TEST_DIR/313-dir/subdir2
> diff --git a/tests/generic/group b/tests/generic/group
> index bd443c1..d5ec8d5 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -115,3 +115,4 @@
>  310 auto
>  311 auto metadata log
>  312 auto quick prealloc enospc
> +313 auto quick acl
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-06-12 19:23   ` Carlos Maiolino
@ 2013-07-08 21:51     ` Ben Myers
  2013-07-11 17:53       ` Carlos Maiolino
  0 siblings, 1 reply; 10+ messages in thread
From: Ben Myers @ 2013-07-08 21:51 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Eric Sandeen, Eric Sandeen, xfs-oss

Hey Carlos & Eric,

On Wed, Jun 12, 2013 at 04:23:20PM -0300, Carlos Maiolino wrote:
> I'm personally working on this bug with Eric, and the test properly trigger the
> bug on 100% of the test runs, but, it should also ensure that
> `irix_sgid_inherit` sysctl is set to 0, if not, the test will fail.
> 
> follow is a suggestion for a better changelog:
> 
> Tests if subdirectories created on the filesystem will properly inherit sgid bit
> when this is set on the parent directory, once the process has the properly
> permissions to create a subdirectory, this, should inherit parent's sgid bit if
> this is set and irix_sgid_inherit sysctl is disabled.

I applied this and tested with Carlos's latest patch:

generic/313      - output mismatch (see /root/xfstests/results/generic/313.out.bad)
    --- tests/generic/313.out   2013-07-08 16:27:41.787710646 -0500
    +++ /root/xfstests/results/generic/313.out.bad      2013-07-08 16:47:46.052683735 -0500
    @@ -1,3 +1,3 @@
     QA output created by 313
    -drwxr-sr-x. TEST_DIR/313-dir/subdir
    +drwxr-sr-x TEST_DIR/313-dir/subdir
     drwxrwsr-x+ TEST_DIR/313-dir/subdir2
     ...
     (Run 'diff -u tests/generic/313.out /root/xfstests/results/generic/313.out.bad' to see the entire diff)

Looks like there could be a problem with ls?  Have you seen that?

Thanks,
Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-07-08 21:51     ` Ben Myers
@ 2013-07-11 17:53       ` Carlos Maiolino
  2013-07-11 18:28         ` Carlos Maiolino
  0 siblings, 1 reply; 10+ messages in thread
From: Carlos Maiolino @ 2013-07-11 17:53 UTC (permalink / raw)
  To: Ben Myers; +Cc: Eric Sandeen, Eric Sandeen, xfs-oss

On Mon, Jul 08, 2013 at 04:51:51PM -0500, Ben Myers wrote:
> Hey Carlos & Eric,
> 
> On Wed, Jun 12, 2013 at 04:23:20PM -0300, Carlos Maiolino wrote:
> > I'm personally working on this bug with Eric, and the test properly trigger the
> > bug on 100% of the test runs, but, it should also ensure that
> > `irix_sgid_inherit` sysctl is set to 0, if not, the test will fail.
> > 
> > follow is a suggestion for a better changelog:
> > 
> > Tests if subdirectories created on the filesystem will properly inherit sgid bit
> > when this is set on the parent directory, once the process has the properly
> > permissions to create a subdirectory, this, should inherit parent's sgid bit if
> > this is set and irix_sgid_inherit sysctl is disabled.
> 
> I applied this and tested with Carlos's latest patch:
> 
> generic/313      - output mismatch (see /root/xfstests/results/generic/313.out.bad)
>     --- tests/generic/313.out   2013-07-08 16:27:41.787710646 -0500
>     +++ /root/xfstests/results/generic/313.out.bad      2013-07-08 16:47:46.052683735 -0500
>     @@ -1,3 +1,3 @@
>      QA output created by 313
>     -drwxr-sr-x. TEST_DIR/313-dir/subdir
>     +drwxr-sr-x TEST_DIR/313-dir/subdir
>      drwxrwsr-x+ TEST_DIR/313-dir/subdir2
>      ...
>      (Run 'diff -u tests/generic/313.out /root/xfstests/results/generic/313.out.bad' to see the entire diff)
> 
> Looks like there could be a problem with ls?  Have you seen that?
> 
> Thanks,
> Ben
> 
Actually I don't think this is a problem, but the way newer `ls` versions are
displaying the object permissions (the 'dot' at the end of the permissions,
indicating there are no extra attributes).

I'm going to take a look on what's going on, but I still believe it's just a
matter of add the 'dot' to the xfstests correct output.

I had this same problem when testing my patch and needed to fix it locally, but
missed to warn you guys, my apologies

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-07-11 17:53       ` Carlos Maiolino
@ 2013-07-11 18:28         ` Carlos Maiolino
  2013-07-11 18:34           ` Eric Sandeen
  0 siblings, 1 reply; 10+ messages in thread
From: Carlos Maiolino @ 2013-07-11 18:28 UTC (permalink / raw)
  To: Ben Myers; +Cc: Eric Sandeen, Eric Sandeen, xfs-oss

> > 
> > generic/313      - output mismatch (see /root/xfstests/results/generic/313.out.bad)
> >     --- tests/generic/313.out   2013-07-08 16:27:41.787710646 -0500
> >     +++ /root/xfstests/results/generic/313.out.bad      2013-07-08 16:47:46.052683735 -0500
> >     @@ -1,3 +1,3 @@
> >      QA output created by 313
> >     -drwxr-sr-x. TEST_DIR/313-dir/subdir
> >     +drwxr-sr-x TEST_DIR/313-dir/subdir
> >      drwxrwsr-x+ TEST_DIR/313-dir/subdir2
> >      ...
> >      (Run 'diff -u tests/generic/313.out /root/xfstests/results/generic/313.out.bad' to see the entire diff)
> > 
> > Looks like there could be a problem with ls?  Have you seen that?
> > 
> > Thanks,
> > Ben
> > 
> Actually I don't think this is a problem, but the way newer `ls` versions are
> displaying the object permissions (the 'dot' at the end of the permissions,
> indicating there are no extra attributes).
> 
> I'm going to take a look on what's going on, but I still believe it's just a
> matter of add the 'dot' to the xfstests correct output.
> 
> I had this same problem when testing my patch and needed to fix it locally, but
> missed to warn you guys, my apologies
> 
Just a matter of information:

>From coreutils:

commit b3677e5e383103bf1764b2c8a9329b1c17934b24
Author: Jim Meyering <meyering@redhat.com>
Date:   Wed Apr 2 22:26:45 2008 +0200

    ls: use '.' (not +) as SELinux-only alt. access flag in ls -l output



So, this test is selinux dependent, it will provide different outputs whether
the system has selinux enabled or not.

Since the test itself creates their own directories, checking if the selinux is
enabled or not and checking the proper output depending on selinux activity
should avoid false positives on this test. I.e. if the selinux is enabled, the
`ls -l` output will print the 'dot' at the end of the permissions, otherwise,
nothing will be printed and Eric's test will pass without problem.

I think this is something worth to mention on xfstests README or some other
documentation, mainly because any kind of test like this, but done with the
TEST_DEV might be even worst since we don't recreate the filesystem while using
TEST_DEV, so, objects there can be created with selinux attrs or not (if created
when selinux is disabled) and have the attributes added later.

I'm probably being too paranoid here talking about the TEST_DEV, but, I thought
it was worth to mention.

Cheers.

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-07-11 18:28         ` Carlos Maiolino
@ 2013-07-11 18:34           ` Eric Sandeen
  2013-07-11 18:53             ` Eric Sandeen
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2013-07-11 18:34 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Eric Sandeen, Ben Myers, xfs-oss

On 7/11/13 1:28 PM, Carlos Maiolino wrote:
>>>
>>> generic/313      - output mismatch (see /root/xfstests/results/generic/313.out.bad)
>>>     --- tests/generic/313.out   2013-07-08 16:27:41.787710646 -0500
>>>     +++ /root/xfstests/results/generic/313.out.bad      2013-07-08 16:47:46.052683735 -0500
>>>     @@ -1,3 +1,3 @@
>>>      QA output created by 313
>>>     -drwxr-sr-x. TEST_DIR/313-dir/subdir
>>>     +drwxr-sr-x TEST_DIR/313-dir/subdir
>>>      drwxrwsr-x+ TEST_DIR/313-dir/subdir2
>>>      ...
>>>      (Run 'diff -u tests/generic/313.out /root/xfstests/results/generic/313.out.bad' to see the entire diff)
>>>
>>> Looks like there could be a problem with ls?  Have you seen that?
>>>
>>> Thanks,
>>> Ben
>>>
>> Actually I don't think this is a problem, but the way newer `ls` versions are
>> displaying the object permissions (the 'dot' at the end of the permissions,
>> indicating there are no extra attributes).
>>
>> I'm going to take a look on what's going on, but I still believe it's just a
>> matter of add the 'dot' to the xfstests correct output.
>>
>> I had this same problem when testing my patch and needed to fix it locally, but
>> missed to warn you guys, my apologies
>>
> Just a matter of information:
> 
> From coreutils:
> 
> commit b3677e5e383103bf1764b2c8a9329b1c17934b24
> Author: Jim Meyering <meyering@redhat.com>
> Date:   Wed Apr 2 22:26:45 2008 +0200
> 
>     ls: use '.' (not +) as SELinux-only alt. access flag in ls -l output
> 
> 
> 
> So, this test is selinux dependent, it will provide different outputs whether
> the system has selinux enabled or not.
> 
> Since the test itself creates their own directories, checking if the selinux is
> enabled or not and checking the proper output depending on selinux activity
> should avoid false positives on this test. I.e. if the selinux is enabled, the
> `ls -l` output will print the 'dot' at the end of the permissions, otherwise,
> nothing will be printed and Eric's test will pass without problem.

Hm, I thought we always mounted with a global selinux context, and therefore
wouldn't get these differences (i.e. no on-disk selinux attrs should be created)

> I think this is something worth to mention on xfstests README or some other
> documentation, mainly because any kind of test like this, but done with the
> TEST_DEV might be even worst since we don't recreate the filesystem while using
> TEST_DEV, so, objects there can be created with selinux attrs or not (if created
> when selinux is disabled) and have the attributes added later.

It'd be better to just add a filter if we need it.

But I'd like to know why we need it, I thought the global context made these
problems go away...

Guess I'll go look...

-Eric

> I'm probably being too paranoid here talking about the TEST_DEV, but, I thought
> it was worth to mention.
> 
> Cheers.
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] xfstests: generic/313, test sgid inheritance on subdirs
  2013-07-11 18:34           ` Eric Sandeen
@ 2013-07-11 18:53             ` Eric Sandeen
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Sandeen @ 2013-07-11 18:53 UTC (permalink / raw)
  To: Carlos Maiolino; +Cc: Eric Sandeen, Ben Myers, xfs-oss

On 7/11/13 1:34 PM, Eric Sandeen wrote:
> On 7/11/13 1:28 PM, Carlos Maiolino wrote:

...

>> Just a matter of information:
>>
>> From coreutils:
>>
>> commit b3677e5e383103bf1764b2c8a9329b1c17934b24
>> Author: Jim Meyering <meyering@redhat.com>
>> Date:   Wed Apr 2 22:26:45 2008 +0200
>>
>>     ls: use '.' (not +) as SELinux-only alt. access flag in ls -l output
>>
>>
>>
>> So, this test is selinux dependent, it will provide different outputs whether
>> the system has selinux enabled or not.
>>
>> Since the test itself creates their own directories, checking if the selinux is
>> enabled or not and checking the proper output depending on selinux activity
>> should avoid false positives on this test. I.e. if the selinux is enabled, the
>> `ls -l` output will print the 'dot' at the end of the permissions, otherwise,
>> nothing will be printed and Eric's test will pass without problem.
> 
> Hm, I thought we always mounted with a global selinux context, and therefore
> wouldn't get these differences (i.e. no on-disk selinux attrs should be created)


Ok, somehow it really is mounted w/o the context when the test executes.
I'm not sure why yet, but fixing that *should* fix the problem, I think.

-Eric

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V3] xfstests: generic/314, test sgid inheritance on subdirs
  2013-05-30 20:02 ` [PATCH V2] " Eric Sandeen
  2013-06-12 19:23   ` Carlos Maiolino
@ 2013-07-12 18:51   ` Eric Sandeen
  2013-07-12 19:21     ` Carlos Maiolino
  1 sibling, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2013-07-12 18:51 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: Carlos Maiolino, xfs-oss

On XFS this test fails today due to some over-eager
SGID removal in xfs core code; ext4 & btrfs pass.
Document it in a test, will send xfs patch in a bit.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

p.s.  I'm going to just flaunt my ignorance and say:
I've never understood acls well.  I'm hoping someone who does
can help me write a better changelog, because this one
is no good at all!

V2: add missing source of "attr" file for _require_acls

V3: use _ls_l to filter out the selinux "." - thanks to Carlos!
    renumber to 314 to make the merge easier

diff --git a/tests/generic/314 b/tests/generic/314
new file mode 100755
index 0000000..3dee609
--- /dev/null
+++ b/tests/generic/314
@@ -0,0 +1,68 @@
+#! /bin/bash
+# FS QA Test No. 314
+#
+# Test SGID inheritance on subdirectories
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2013 Red Hat, Inc.  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
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+    cd /
+    rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+. ./common/attr
+
+# real QA test starts here
+_supported_fs generic
+_require_acls
+_require_user
+_need_to_be_root
+
+rm -rf $TEST_DIR/$seq-dir
+
+# Make dir owned by qa user, and an unrelated group:
+mkdir $TEST_DIR/$seq-dir
+chown $qa_user:12345 $TEST_DIR/$seq-dir
+
+# Make parent dir sgid
+chmod 2775 $TEST_DIR/$seq-dir
+
+# Make subdirs before & after acl set
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir"
+su $qa_user -c "setfacl -m u:$qa_user:rwx,d:u:$qa_user:rwx $TEST_DIR/$seq-dir"
+su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir2"
+
+# Both subdirs should have inherited sgid
+_ls_l $TEST_DIR/$seq-dir/ | _filter_test_dir | awk '{print $1,$NF}'
+
+status=0
+exit
diff --git a/tests/generic/314.out b/tests/generic/314.out
new file mode 100644
index 0000000..326a929
--- /dev/null
+++ b/tests/generic/314.out
@@ -0,0 +1,3 @@
+QA output created by 314
+drwxr-sr-x. TEST_DIR/314-dir/subdir
+drwxrwsr-x+ TEST_DIR/314-dir/subdir2
diff --git a/tests/generic/group b/tests/generic/group
index bd443c1..d5ec8d5 100644
--- a/tests/generic/group
+++ b/tests/generic/group
@@ -115,3 +115,4 @@
 310 auto
 311 auto metadata log
 312 auto quick prealloc enospc
+314 auto quick acl

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V3] xfstests: generic/314, test sgid inheritance on subdirs
  2013-07-12 18:51   ` [PATCH V3] xfstests: generic/314, " Eric Sandeen
@ 2013-07-12 19:21     ` Carlos Maiolino
  0 siblings, 0 replies; 10+ messages in thread
From: Carlos Maiolino @ 2013-07-12 19:21 UTC (permalink / raw)
  To: Eric Sandeen; +Cc: xfs-oss

Hi Eric,

the 314.out file is wrong, it should have:

total 4
drwxr-sr-x subdir
drwxrwxr-x+ subdir2

On Fri, Jul 12, 2013 at 01:51:35PM -0500, Eric Sandeen wrote:
> On XFS this test fails today due to some over-eager
> SGID removal in xfs core code; ext4 & btrfs pass.
> Document it in a test, will send xfs patch in a bit.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> p.s.  I'm going to just flaunt my ignorance and say:
> I've never understood acls well.  I'm hoping someone who does
> can help me write a better changelog, because this one
> is no good at all!
> 
> V2: add missing source of "attr" file for _require_acls
> 
> V3: use _ls_l to filter out the selinux "." - thanks to Carlos!
>     renumber to 314 to make the merge easier
> 
> diff --git a/tests/generic/314 b/tests/generic/314
> new file mode 100755
> index 0000000..3dee609
> --- /dev/null
> +++ b/tests/generic/314
> @@ -0,0 +1,68 @@
> +#! /bin/bash
> +# FS QA Test No. 314
> +#
> +# Test SGID inheritance on subdirectories
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2013 Red Hat, Inc.  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
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +    cd /
> +    rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +. ./common/attr
> +
> +# real QA test starts here
> +_supported_fs generic
> +_require_acls
> +_require_user
> +_need_to_be_root
> +
> +rm -rf $TEST_DIR/$seq-dir
> +
> +# Make dir owned by qa user, and an unrelated group:
> +mkdir $TEST_DIR/$seq-dir
> +chown $qa_user:12345 $TEST_DIR/$seq-dir
> +
> +# Make parent dir sgid
> +chmod 2775 $TEST_DIR/$seq-dir
> +
> +# Make subdirs before & after acl set
> +su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir"
> +su $qa_user -c "setfacl -m u:$qa_user:rwx,d:u:$qa_user:rwx $TEST_DIR/$seq-dir"
> +su $qa_user -c "mkdir $TEST_DIR/$seq-dir/subdir2"
> +
> +# Both subdirs should have inherited sgid
> +_ls_l $TEST_DIR/$seq-dir/ | _filter_test_dir | awk '{print $1,$NF}'
> +
> +status=0
> +exit
> diff --git a/tests/generic/314.out b/tests/generic/314.out
> new file mode 100644
> index 0000000..326a929
> --- /dev/null
> +++ b/tests/generic/314.out
> @@ -0,0 +1,3 @@
> +QA output created by 314
> +drwxr-sr-x. TEST_DIR/314-dir/subdir
> +drwxrwsr-x+ TEST_DIR/314-dir/subdir2
> diff --git a/tests/generic/group b/tests/generic/group
> index bd443c1..d5ec8d5 100644
> --- a/tests/generic/group
> +++ b/tests/generic/group
> @@ -115,3 +115,4 @@
>  310 auto
>  311 auto metadata log
>  312 auto quick prealloc enospc
> +314 auto quick acl
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-07-12 19:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-29 22:30 [PATCH] xfstests: generic/313, test sgid inheritance on subdirs Eric Sandeen
2013-05-30 20:02 ` [PATCH V2] " Eric Sandeen
2013-06-12 19:23   ` Carlos Maiolino
2013-07-08 21:51     ` Ben Myers
2013-07-11 17:53       ` Carlos Maiolino
2013-07-11 18:28         ` Carlos Maiolino
2013-07-11 18:34           ` Eric Sandeen
2013-07-11 18:53             ` Eric Sandeen
2013-07-12 18:51   ` [PATCH V3] xfstests: generic/314, " Eric Sandeen
2013-07-12 19:21     ` Carlos Maiolino

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.