| Message ID | 1487842966-30986-7-git-send-email-xzhou@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@redhat.com> wrote: > Even timestamp change by touch. > > CC: Miklos Szeredi <mszeredi@redhat.com> > Signed-off-by: Xiong Zhou <xzhou@redhat.com> > --- > tests/overlay/027 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/overlay/027.out | 2 ++ > tests/overlay/group | 1 + > 3 files changed, 90 insertions(+) > create mode 100755 tests/overlay/027 > create mode 100644 tests/overlay/027.out > > diff --git a/tests/overlay/027 b/tests/overlay/027 > new file mode 100755 > index 0000000..4bb0329 > --- /dev/null > +++ b/tests/overlay/027 > @@ -0,0 +1,87 @@ > +#! /bin/bash > +# FS QA Test 027 > +# > +# If underlying upper file is immutable, it should stays > +# untouchable in the overlayfs mount. > +# > +# Kernel commit below fixed it. > +# f2b20f6ee842 vfs: move permission checking into ... > +# > +# This reproducer was originally written by > +# Miklos Szeredi <mszeredi@redhat.com> > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2017 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 / > + $CHATTR_PROG -i $upperdir/foo > + rm -f $tmp.* > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# remove previous $seqres.full before test > +rm -f $seqres.full > + > +# real QA test starts here > + > +# Modify as appropriate. > +_supported_fs overlay > +_supported_os Linux > +_require_scratch > +_require_chattr > + Eryu, A way-post-merge bug spotting: Should be "_require_chattr i". _require_chattr alone makes no sense (and should probably be reported as an error) I may get to send a fix, but not too soon.. > +# Remove all files from previous tests > +_scratch_mkfs > + > +# Preparing immutable file > +upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR > +mkdir -p $upperdir > +touch $upperdir/foo > +$CHATTR_PROG +i $upperdir/foo > + > +# Mounting overlay > +_scratch_mount > + > +# Touching immutable file in overlay, should fail. > +# Not filtering output here because of errno returned > +# varies among kernel versions: > +# touch: setting times of 'foo': Operation not permitted > +# touch: cannot touch 'foo': Permission denied > +if touch $SCRATCH_MNT/foo > /dev/null 2>&1 ; then > + echo "Test Fail, you can't change an immutable file" > +else > + echo "Silence is golden" > +fi > + > +# success, all done > +status=0 > +exit > diff --git a/tests/overlay/027.out b/tests/overlay/027.out > new file mode 100644 > index 0000000..ef78cda > --- /dev/null > +++ b/tests/overlay/027.out > @@ -0,0 +1,2 @@ > +QA output created by 027 > +Silence is golden > diff --git a/tests/overlay/group b/tests/overlay/group > index 92afa8d..4db6751 100644 > --- a/tests/overlay/group > +++ b/tests/overlay/group > @@ -29,3 +29,4 @@ > 024 auto quick > 025 auto quick attr > 026 auto attr quick > +027 auto quick perms > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 4, 2017 at 2:19 PM, Amir Goldstein <amir73il@gmail.com> wrote: > On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@redhat.com> wrote: [...] >> + >> +# Modify as appropriate. >> +_supported_fs overlay >> +_supported_os Linux >> +_require_scratch >> +_require_chattr >> + > > Eryu, > > A way-post-merge bug spotting: > Should be "_require_chattr i". > _require_chattr alone makes no sense (and should probably be reported > as an error) > > I may get to send a fix, but not too soon.. > Apparently, _require_chattr with no arguments does do what it is expected to do, or as David wrote in the new documentation patch: "The test requires that the chattr command be available and supported by the $TEST_DEV filesystem" But it appears to be doing the right thing by coincidence?? Without any arguments to _require_chattr, it ends up executing chattr + $file and chattr - file The former, surprisingly does do FS_IOC_GETFLAGS and FS_IOC_SETFLAGS on the file, so the check is correct. The latter spews this error into full output "Must use '-v', =, - or +" and ignored the failure - again correct, but seems unintentional. I wonder if we should forbid the use of _require_chattr with no args? It's only user right now is test overlay/027 and that seems like a bug (test should _require_chattr i). -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 04, 2017 at 07:31:46PM +0300, Amir Goldstein wrote: > On Tue, Apr 4, 2017 at 2:19 PM, Amir Goldstein <amir73il@gmail.com> wrote: > > On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@redhat.com> wrote: > [...] > >> + > >> +# Modify as appropriate. > >> +_supported_fs overlay > >> +_supported_os Linux > >> +_require_scratch > >> +_require_chattr > >> + > > > > Eryu, > > > > A way-post-merge bug spotting: > > Should be "_require_chattr i". > > _require_chattr alone makes no sense (and should probably be reported > > as an error) > > > > I may get to send a fix, but not too soon.. > > > > Apparently, _require_chattr with no arguments does do what it is expected to do, > or as David wrote in the new documentation patch: > "The test requires that the chattr command be available and supported by > the $TEST_DEV filesystem" > > But it appears to be doing the right thing by coincidence?? > Without any arguments to _require_chattr, it ends up executing > > chattr + $file > > and > > chattr - file > > The former, surprisingly does do FS_IOC_GETFLAGS and FS_IOC_SETFLAGS > on the file, so the check is correct. > > The latter spews this error into full output "Must use '-v', =, - or +" > and ignored the failure - again correct, but seems unintentional. > > I wonder if we should forbid the use of _require_chattr with no args? > It's only user right now is test overlay/027 and that seems like a bug > (test should _require_chattr i). Yes, it's a bug, my bad. Thanks for catching this, good job! -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Sorry for the late response, was in public holiday] On Tue, Apr 04, 2017 at 07:31:46PM +0300, Amir Goldstein wrote: > On Tue, Apr 4, 2017 at 2:19 PM, Amir Goldstein <amir73il@gmail.com> wrote: > > On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@redhat.com> wrote: > [...] > >> + > >> +# Modify as appropriate. > >> +_supported_fs overlay > >> +_supported_os Linux > >> +_require_scratch > >> +_require_chattr > >> + > > > > Eryu, > > > > A way-post-merge bug spotting: > > Should be "_require_chattr i". > > _require_chattr alone makes no sense (and should probably be reported > > as an error) > > > > I may get to send a fix, but not too soon.. > > > > Apparently, _require_chattr with no arguments does do what it is expected to do, > or as David wrote in the new documentation patch: > "The test requires that the chattr command be available and supported by > the $TEST_DEV filesystem" > > But it appears to be doing the right thing by coincidence?? > Without any arguments to _require_chattr, it ends up executing > > chattr + $file > > and > > chattr - file > > The former, surprisingly does do FS_IOC_GETFLAGS and FS_IOC_SETFLAGS > on the file, so the check is correct. > > The latter spews this error into full output "Must use '-v', =, - or +" > and ignored the failure - again correct, but seems unintentional. > > I wonder if we should forbid the use of _require_chattr with no args? I agreed. Do you mind sending a fix? > It's only user right now is test overlay/027 and that seems like a bug > (test should _require_chattr i). Agreed. Xiong has acknowledged the bug, hopefully he will send a fix. Thanks for finding this! Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 5, 2017 at 5:50 AM, Eryu Guan <eguan@redhat.com> wrote: > [Sorry for the late response, was in public holiday] > > On Tue, Apr 04, 2017 at 07:31:46PM +0300, Amir Goldstein wrote: >> On Tue, Apr 4, 2017 at 2:19 PM, Amir Goldstein <amir73il@gmail.com> wrote: >> > On Thu, Feb 23, 2017 at 11:42 AM, Xiong Zhou <xzhou@redhat.com> wrote: >> [...] >> >> + >> >> +# Modify as appropriate. >> >> +_supported_fs overlay >> >> +_supported_os Linux >> >> +_require_scratch >> >> +_require_chattr >> >> + >> > >> > Eryu, >> > >> > A way-post-merge bug spotting: >> > Should be "_require_chattr i". >> > _require_chattr alone makes no sense (and should probably be reported >> > as an error) >> > >> > I may get to send a fix, but not too soon.. >> > >> >> Apparently, _require_chattr with no arguments does do what it is expected to do, >> or as David wrote in the new documentation patch: >> "The test requires that the chattr command be available and supported by >> the $TEST_DEV filesystem" >> >> But it appears to be doing the right thing by coincidence?? >> Without any arguments to _require_chattr, it ends up executing >> >> chattr + $file >> >> and >> >> chattr - file >> >> The former, surprisingly does do FS_IOC_GETFLAGS and FS_IOC_SETFLAGS >> on the file, so the check is correct. >> >> The latter spews this error into full output "Must use '-v', =, - or +" >> and ignored the failure - again correct, but seems unintentional. >> >> I wonder if we should forbid the use of _require_chattr with no args? > > I agreed. Do you mind sending a fix? Fix sent. > >> It's only user right now is test overlay/027 and that seems like a bug >> (test should _require_chattr i). > > Agreed. Xiong has acknowledged the bug, hopefully he will send a fix. > While at it, I noticed that overlay/027 only tests immutable regular files but overlay actually fails some immutable directory tests and append-only files (and dirs) test, so created overlay/030 to test those (it is failing). Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/overlay/027 b/tests/overlay/027 new file mode 100755 index 0000000..4bb0329 --- /dev/null +++ b/tests/overlay/027 @@ -0,0 +1,87 @@ +#! /bin/bash +# FS QA Test 027 +# +# If underlying upper file is immutable, it should stays +# untouchable in the overlayfs mount. +# +# Kernel commit below fixed it. +# f2b20f6ee842 vfs: move permission checking into ... +# +# This reproducer was originally written by +# Miklos Szeredi <mszeredi@redhat.com> +# +#----------------------------------------------------------------------- +# Copyright (c) 2017 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 / + $CHATTR_PROG -i $upperdir/foo + rm -f $tmp.* +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# remove previous $seqres.full before test +rm -f $seqres.full + +# real QA test starts here + +# Modify as appropriate. +_supported_fs overlay +_supported_os Linux +_require_scratch +_require_chattr + +# Remove all files from previous tests +_scratch_mkfs + +# Preparing immutable file +upperdir=$SCRATCH_DEV/$OVERLAY_UPPER_DIR +mkdir -p $upperdir +touch $upperdir/foo +$CHATTR_PROG +i $upperdir/foo + +# Mounting overlay +_scratch_mount + +# Touching immutable file in overlay, should fail. +# Not filtering output here because of errno returned +# varies among kernel versions: +# touch: setting times of 'foo': Operation not permitted +# touch: cannot touch 'foo': Permission denied +if touch $SCRATCH_MNT/foo > /dev/null 2>&1 ; then + echo "Test Fail, you can't change an immutable file" +else + echo "Silence is golden" +fi + +# success, all done +status=0 +exit diff --git a/tests/overlay/027.out b/tests/overlay/027.out new file mode 100644 index 0000000..ef78cda --- /dev/null +++ b/tests/overlay/027.out @@ -0,0 +1,2 @@ +QA output created by 027 +Silence is golden diff --git a/tests/overlay/group b/tests/overlay/group index 92afa8d..4db6751 100644 --- a/tests/overlay/group +++ b/tests/overlay/group @@ -29,3 +29,4 @@ 024 auto quick 025 auto quick attr 026 auto attr quick +027 auto quick perms
Even timestamp change by touch. CC: Miklos Szeredi <mszeredi@redhat.com> Signed-off-by: Xiong Zhou <xzhou@redhat.com> --- tests/overlay/027 | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/overlay/027.out | 2 ++ tests/overlay/group | 1 + 3 files changed, 90 insertions(+) create mode 100755 tests/overlay/027 create mode 100644 tests/overlay/027.out