| Message ID | 20200910194355.5977-1-fllinden@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] common/attr: make _require_attrs more fine-grained | expand |
On Thu, Sep 10, 2020 at 07:43:53PM +0000, Frank van der Linden wrote: > Filesystems may not support all xattr types. But, _require_attr assumes > that being able to use "user" namespace xattrs means that all namespaces > ("trusted", "system", etc) are supported. This breaks on NFS, that only > supports the "user" namespace, and a few cases in the "system" namespace. > > Change _require_attrs to optionally take namespace arguments that specify > the namespaces to check for. The default behavior (no arguments) is still > to check for the "user" namespace only. > > Signed-off-by: Frank van der Linden <fllinden@amazon.com> This patchset looks great to me, thanks! Some minor nits below (and I've fixed them on commit, so there's no need to resend :) > --- > common/attr | 49 +++++++++++++++++++++++++++++++------------------ > 1 file changed, 31 insertions(+), 18 deletions(-) > > diff --git a/common/attr b/common/attr > index 20049de0..c60cb6ed 100644 > --- a/common/attr > +++ b/common/attr > @@ -175,30 +175,43 @@ _list_acl() > > _require_attrs() > { > + local args > + local nsp > + > + if [ $# -eq 0 ]; > + then We prefer the following coding style in fstests if [ $# -eq 0 ]; then args="user" else args="$*" fi > + args="user" > + else > + args="$*" > + fi And you've almost re-written the whole _require_attrs(), it's better to use tab as indention instead of 4 spaces (we're in the (very slow) progress converting all 4-spaces indention to tab, except there're old code using 4-spaces around). > + > [ -n "$ATTR_PROG" ] || _notrun "attr command not found" > [ -n "$GETFATTR_PROG" ] || _notrun "getfattr command not found" > [ -n "$SETFATTR_PROG" ] || _notrun "setfattr command not found" > > - # > - # Test if chacl is able to write an attribute on the target filesystems. > - # On really old kernels the system calls might not be implemented at all, > - # but the more common case is that the tested filesystem simply doesn't > - # support attributes. Note that we can't simply list attributes as > - # various security modules generate synthetic attributes not actually > - # stored on disk. > - # > - touch $TEST_DIR/syscalltest > - attr -s "user.xfstests" -V "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1 > - cat $TEST_DIR/syscalltest.out >> $seqres.full > + for nsp in $args > + do Same here, use the format below for nsp in $args; do <do things here> done Thanks, Eryu > + # > + # Test if chacl is able to write an attribute on the target filesystems. > + # On really old kernels the system calls might not be implemented at all, > + # but the more common case is that the tested filesystem simply doesn't > + # support attributes. Note that we can't simply list attributes as > + # various security modules generate synthetic attributes not actually > + # stored on disk. > + # > + touch $TEST_DIR/syscalltest > + $SETFATTR_PROG -n "$nsp.xfstests" -v "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1 > + cat $TEST_DIR/syscalltest.out >> $seqres.full > > - if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then > - _notrun "kernel does not support attrs" > - fi > - if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then > - _notrun "attrs not supported by this filesystem type: $FSTYP" > - fi > + if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then > + _notrun "kernel does not support attrs" > + fi > + if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then > + _notrun "attr namespace $nsp not supported by this filesystem type: $FSTYP" > + fi > > - rm -f $TEST_DIR/syscalltest.out > + rm -f $TEST_DIR/syscalltest.out > + done > } > > _require_attr_v1() > -- > 2.16.6
diff --git a/common/attr b/common/attr index 20049de0..c60cb6ed 100644 --- a/common/attr +++ b/common/attr @@ -175,30 +175,43 @@ _list_acl() _require_attrs() { + local args + local nsp + + if [ $# -eq 0 ]; + then + args="user" + else + args="$*" + fi + [ -n "$ATTR_PROG" ] || _notrun "attr command not found" [ -n "$GETFATTR_PROG" ] || _notrun "getfattr command not found" [ -n "$SETFATTR_PROG" ] || _notrun "setfattr command not found" - # - # Test if chacl is able to write an attribute on the target filesystems. - # On really old kernels the system calls might not be implemented at all, - # but the more common case is that the tested filesystem simply doesn't - # support attributes. Note that we can't simply list attributes as - # various security modules generate synthetic attributes not actually - # stored on disk. - # - touch $TEST_DIR/syscalltest - attr -s "user.xfstests" -V "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1 - cat $TEST_DIR/syscalltest.out >> $seqres.full + for nsp in $args + do + # + # Test if chacl is able to write an attribute on the target filesystems. + # On really old kernels the system calls might not be implemented at all, + # but the more common case is that the tested filesystem simply doesn't + # support attributes. Note that we can't simply list attributes as + # various security modules generate synthetic attributes not actually + # stored on disk. + # + touch $TEST_DIR/syscalltest + $SETFATTR_PROG -n "$nsp.xfstests" -v "attr" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1 + cat $TEST_DIR/syscalltest.out >> $seqres.full - if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then - _notrun "kernel does not support attrs" - fi - if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then - _notrun "attrs not supported by this filesystem type: $FSTYP" - fi + if grep -q 'Function not implemented' $TEST_DIR/syscalltest.out; then + _notrun "kernel does not support attrs" + fi + if grep -q 'Operation not supported' $TEST_DIR/syscalltest.out; then + _notrun "attr namespace $nsp not supported by this filesystem type: $FSTYP" + fi - rm -f $TEST_DIR/syscalltest.out + rm -f $TEST_DIR/syscalltest.out + done } _require_attr_v1()
Filesystems may not support all xattr types. But, _require_attr assumes that being able to use "user" namespace xattrs means that all namespaces ("trusted", "system", etc) are supported. This breaks on NFS, that only supports the "user" namespace, and a few cases in the "system" namespace. Change _require_attrs to optionally take namespace arguments that specify the namespaces to check for. The default behavior (no arguments) is still to check for the "user" namespace only. Signed-off-by: Frank van der Linden <fllinden@amazon.com> --- common/attr | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-)