diff mbox series

[v2,2/3] generic/513: limit to filesystems that support capabilities

Message ID 20230824-fixes-v2-2-d60c2faf1057@kernel.org (mailing list archive)
State New, archived
Headers show
Series fstests: add appropriate checks for fs features for some tests | expand

Commit Message

Jeff Layton Aug. 24, 2023, 4:44 p.m. UTC
This test requires being able to set file capabilities which some
filesystems (namely NFS) do not support. Add a _require_setcap test
and only run it on filesystems that pass it.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 common/rc         | 13 +++++++++++++
 tests/generic/513 |  1 +
 2 files changed, 14 insertions(+)

Comments

Zorro Lang Aug. 25, 2023, 2:11 p.m. UTC | #1
On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote:
> This test requires being able to set file capabilities which some
> filesystems (namely NFS) do not support. Add a _require_setcap test
> and only run it on filesystems that pass it.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  common/rc         | 13 +++++++++++++
>  tests/generic/513 |  1 +
>  2 files changed, 14 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index 5c4429ed0425..33e74d20c28b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5048,6 +5048,19 @@ _require_mknod()
>  	rm -f $TEST_DIR/$seq.null
>  }
>  
> +_require_setcap()
> +{
> +	local testfile=$TEST_DIR/setcaptest.$$
> +
> +	touch $testfile
> +	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1

Actually we talked about the capabilities checking helper last year, as below:

https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/

As you bring this discussion back, how about the _require_capabilities() in
above link?

Thanks,
Zorro

> +	if grep -q 'Operation not supported' $testfile.out; then
> +	  _notrun "File capabilities are not supported on this filesystem"
> +	fi
> +
> +	rm -f $testfile $testfile.out
> +}
> +
>  _getcap()
>  {
>  	$GETCAP_PROG "$@" | _filter_getcap
> diff --git a/tests/generic/513 b/tests/generic/513
> index dc082787ae4e..52f9eb916b4a 100755
> --- a/tests/generic/513
> +++ b/tests/generic/513
> @@ -18,6 +18,7 @@ _supported_fs generic
>  _require_scratch_reflink
>  _require_command "$GETCAP_PROG" getcap
>  _require_command "$SETCAP_PROG" setcap
> +_require_setcap
>  
>  _scratch_mkfs >>$seqres.full 2>&1
>  _scratch_mount
> 
> -- 
> 2.41.0
>
Jeff Layton Aug. 25, 2023, 3:02 p.m. UTC | #2
On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote:
> On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote:
> > This test requires being able to set file capabilities which some
> > filesystems (namely NFS) do not support. Add a _require_setcap test
> > and only run it on filesystems that pass it.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> >  common/rc         | 13 +++++++++++++
> >  tests/generic/513 |  1 +
> >  2 files changed, 14 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index 5c4429ed0425..33e74d20c28b 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -5048,6 +5048,19 @@ _require_mknod()
> >  	rm -f $TEST_DIR/$seq.null
> >  }
> >  
> > +_require_setcap()
> > +{
> > +	local testfile=$TEST_DIR/setcaptest.$$
> > +
> > +	touch $testfile
> > +	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1
> 
> Actually we talked about the capabilities checking helper last year, as below:
> 
> https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/
> 
> As you bring this discussion back, how about the _require_capabilities() in
> above link?
> 

I was testing a similar patch, but your version looks better. Should I
drop mine and you re-post yours?

Thanks,
--
Jeff Layton <jlayton@kernel.org>
Zorro Lang Aug. 27, 2023, 12:45 p.m. UTC | #3
On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote:
> On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote:
> > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote:
> > > This test requires being able to set file capabilities which some
> > > filesystems (namely NFS) do not support. Add a _require_setcap test
> > > and only run it on filesystems that pass it.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  common/rc         | 13 +++++++++++++
> > >  tests/generic/513 |  1 +
> > >  2 files changed, 14 insertions(+)
> > > 
> > > diff --git a/common/rc b/common/rc
> > > index 5c4429ed0425..33e74d20c28b 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -5048,6 +5048,19 @@ _require_mknod()
> > >  	rm -f $TEST_DIR/$seq.null
> > >  }
> > >  
> > > +_require_setcap()
> > > +{
> > > +	local testfile=$TEST_DIR/setcaptest.$$
> > > +
> > > +	touch $testfile
> > > +	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1
> > 
> > Actually we talked about the capabilities checking helper last year, as below:
> > 
> > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/
> > 
> > As you bring this discussion back, how about the _require_capabilities() in
> > above link?
> > 
> 
> I was testing a similar patch, but your version looks better. Should I
> drop mine and you re-post yours?

Actually we decided to use `_require_attrs security`, rather than a new
_require_capabilities() helper. We need a chance/requirement to add
that helper (when a test case really need it).

So I hope know is `_require_attrs security` enough for you? Or you really
need a specific _require_capabilities helper?

Thanks,
Zorro

> 
> Thanks,
> --
> Jeff Layton <jlayton@kernel.org>
>
Jeff Layton Aug. 27, 2023, 1:43 p.m. UTC | #4
On Sun, 2023-08-27 at 20:45 +0800, Zorro Lang wrote:
> On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote:
> > On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote:
> > > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote:
> > > > This test requires being able to set file capabilities which some
> > > > filesystems (namely NFS) do not support. Add a _require_setcap test
> > > > and only run it on filesystems that pass it.
> > > > 
> > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > ---
> > > >  common/rc         | 13 +++++++++++++
> > > >  tests/generic/513 |  1 +
> > > >  2 files changed, 14 insertions(+)
> > > > 
> > > > diff --git a/common/rc b/common/rc
> > > > index 5c4429ed0425..33e74d20c28b 100644
> > > > --- a/common/rc
> > > > +++ b/common/rc
> > > > @@ -5048,6 +5048,19 @@ _require_mknod()
> > > >  	rm -f $TEST_DIR/$seq.null
> > > >  }
> > > >  
> > > > +_require_setcap()
> > > > +{
> > > > +	local testfile=$TEST_DIR/setcaptest.$$
> > > > +
> > > > +	touch $testfile
> > > > +	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1
> > > 
> > > Actually we talked about the capabilities checking helper last year, as below:
> > > 
> > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/
> > > 
> > > As you bring this discussion back, how about the _require_capabilities() in
> > > above link?
> > > 
> > 
> > I was testing a similar patch, but your version looks better. Should I
> > drop mine and you re-post yours?
> 
> Actually we decided to use `_require_attrs security`, rather than a new
> _require_capabilities() helper. We need a chance/requirement to add
> that helper (when a test case really need it).
> 
> So I hope know is `_require_attrs security` enough for you? Or you really
> need a specific _require_capabilities helper?
> 
> Thanks,
> Zorro
> 
> 


Yeah, it looks like that should work:

    generic/513       [not run] attr namespace security not supported by this filesystem type: nfs

I'll plan to respin this patch to add that instead.

Thanks!
Zorro Lang Aug. 28, 2023, 2:35 p.m. UTC | #5
On Sun, Aug 27, 2023 at 09:43:41AM -0400, Jeff Layton wrote:
> On Sun, 2023-08-27 at 20:45 +0800, Zorro Lang wrote:
> > On Fri, Aug 25, 2023 at 11:02:40AM -0400, Jeff Layton wrote:
> > > On Fri, 2023-08-25 at 22:11 +0800, Zorro Lang wrote:
> > > > On Thu, Aug 24, 2023 at 12:44:18PM -0400, Jeff Layton wrote:
> > > > > This test requires being able to set file capabilities which some
> > > > > filesystems (namely NFS) do not support. Add a _require_setcap test
> > > > > and only run it on filesystems that pass it.
> > > > > 
> > > > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > > > ---
> > > > >  common/rc         | 13 +++++++++++++
> > > > >  tests/generic/513 |  1 +
> > > > >  2 files changed, 14 insertions(+)
> > > > > 
> > > > > diff --git a/common/rc b/common/rc
> > > > > index 5c4429ed0425..33e74d20c28b 100644
> > > > > --- a/common/rc
> > > > > +++ b/common/rc
> > > > > @@ -5048,6 +5048,19 @@ _require_mknod()
> > > > >  	rm -f $TEST_DIR/$seq.null
> > > > >  }
> > > > >  
> > > > > +_require_setcap()
> > > > > +{
> > > > > +	local testfile=$TEST_DIR/setcaptest.$$
> > > > > +
> > > > > +	touch $testfile
> > > > > +	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1
> > > > 
> > > > Actually we talked about the capabilities checking helper last year, as below:
> > > > 
> > > > https://lore.kernel.org/fstests/20220323023845.saj5en74km7aibdx@zlang-mailbox/
> > > > 
> > > > As you bring this discussion back, how about the _require_capabilities() in
> > > > above link?
> > > > 
> > > 
> > > I was testing a similar patch, but your version looks better. Should I
> > > drop mine and you re-post yours?
> > 
> > Actually we decided to use `_require_attrs security`, rather than a new
> > _require_capabilities() helper. We need a chance/requirement to add
> > that helper (when a test case really need it).
> > 
> > So I hope know is `_require_attrs security` enough for you? Or you really
> > need a specific _require_capabilities helper?
> > 
> > Thanks,
> > Zorro
> > 
> > 
> 
> 
> Yeah, it looks like that should work:
> 
>     generic/513       [not run] attr namespace security not supported by this filesystem type: nfs
> 
> I'll plan to respin this patch to add that instead.

Thanks:) I saw you've sent a V3:
[PATCH fstests v3 0/2] fstests: add appropriate checks for fs features for some tests

so will you sent a V4 contains 3 patches?

Thanks,
Zorro

> 
> Thanks!
> -- 
> Jeff Layton <jlayton@kernel.org>
>
diff mbox series

Patch

diff --git a/common/rc b/common/rc
index 5c4429ed0425..33e74d20c28b 100644
--- a/common/rc
+++ b/common/rc
@@ -5048,6 +5048,19 @@  _require_mknod()
 	rm -f $TEST_DIR/$seq.null
 }
 
+_require_setcap()
+{
+	local testfile=$TEST_DIR/setcaptest.$$
+
+	touch $testfile
+	$SETCAP_PROG "cap_sys_module=p" $testfile > $testfile.out 2>&1
+	if grep -q 'Operation not supported' $testfile.out; then
+	  _notrun "File capabilities are not supported on this filesystem"
+	fi
+
+	rm -f $testfile $testfile.out
+}
+
 _getcap()
 {
 	$GETCAP_PROG "$@" | _filter_getcap
diff --git a/tests/generic/513 b/tests/generic/513
index dc082787ae4e..52f9eb916b4a 100755
--- a/tests/generic/513
+++ b/tests/generic/513
@@ -18,6 +18,7 @@  _supported_fs generic
 _require_scratch_reflink
 _require_command "$GETCAP_PROG" getcap
 _require_command "$SETCAP_PROG" setcap
+_require_setcap
 
 _scratch_mkfs >>$seqres.full 2>&1
 _scratch_mount