issue with unsetting 'arch' flag

Alexander Best arundel at freebsd.org
Thu Oct 7 10:36:01 UTC 2010


On Wed Oct  6 10, Garrett Cooper wrote:
> On Wed, Oct 6, 2010 at 4:03 PM, Garrett Cooper <gcooper at freebsd.org> wrote:
> > On Wed, Oct 6, 2010 at 3:01 PM, Sergey Kandaurov <pluknet at gmail.com> wrote:
> >> On 6 October 2010 23:38, Alexander Best <arundel at freebsd.org> wrote:
> >>> On Wed Oct  6 10, Garrett Cooper wrote:
> >>>> On Wed, Oct 6, 2010 at 10:35 AM, Alexander Best <arundel at freebsd.org> wrote:
> >>>> > On Wed Oct  6 10, Garrett Cooper wrote:
> >>>> >> On Tue, Oct 5, 2010 at 4:50 PM, Alexander Best <arundel at freebsd.org> wrote:
> >>>> >> > hi there,
> >>>> >> >
> >>>> >> > i think the following example shows the problem better than a long explanation:
> >>>> >> >
> >>>> >> > `touch ftest && chflags arch ftest && chflags -vv 0 ftest`.
> >>>> >> >  ^^non-root     ^^root                ^^non-root
> >>>> >> >
> >>>> >> > chflags claims to have cleared the 'arch' flag (which should be impossible as
> >>>> >> > non-root user), but indeed has done nothing.
> >>>> >> >
> >>>> >> > i've tried the same with 'sappnd' and that works as can be expected.
> >>>> >> >
> >>>> >> > The issue was confirmed to exist in HEAD (me), stable/8 (pgollucc1, jpaetzel)
> >>>> >> > and stable/7 (nox).
> >>>> >> > On stable/6 it does NOT exist (jpaetzel). chflags properly fails with EPERM.
> >>>> >>
> >>>> >>     Fails for me when I call the syscall directly, as I would expect,
> >>>> >> and passes when I'm superuser:
> >>>> >>
> >>>> >> $ ./test_chflags
> >>>> >> (uid, euid) = (1000, 1000)
> >>>> >> test_chflags: chflags: Operation not permitted
> >>>> >> test_chflags: lchflags: Operation not permitted
> >>>> >> $ sudo ./test_chflags
> >>>> >> (uid, euid) = (0, 0)
> >>>> >>
> >>>> >>     According to my basic inspection in strtofflags
> >>>> >> (.../lib/libc/gen/strtofflags.c), it works as well.
> >>>> >>     And last but not least, executing the commands directly on the CLI work:
> >>>> >>
> >>>> >> $ tmpfile=`mktemp /tmp/chflags.XXXXXX`
> >>>> >> $ chflags arch $tmpfile
> >>>> >> chflags: /tmp/chflags.nQm1IL: Operation not permitted
> >>>> >> $ rm $tmpfile
> >>>> >> $ tmpfile=`mktemp /tmp/chflags.XXXXXX`
> >>>> >> $ sudo chflags arch $tmpfile
> >>>> >> $ sudo chflags noarch $tmpfile
> >>>> >> $ rm $tmpfile
> >>>> >
> >>>> > thanks for your test app and helping out with this problem. i'm not sure
> >>>> > however you understood the problem. probably i didn't explain it right:
> >>>> >
> >>>> > $ sudo rm -d /tmp/chflags.XXXXXX
> >>>> > $ tmpfile=`mktemp /tmp/chflags.XXXXXX`
> >>>> > $ sudo chflags arch $tmpfile
> >>>> > $ chflags noarch $tmpfile
> >>>> >
> >>>> > is what's causing the problem. the last chflags call should fail, but it
> >>>> > doesn't.
> >>>>
> >>>> Sorry... my CLI based example was stupid. I meant:
> >>>>
> >>>> $ tmpfile=`mktemp /tmp/chflags.XXXXXX`
> >>>> $ chflags arch $tmpfile
> >>>> chflags: /tmp/chflags.V2NpXR: Operation not permitted
> >>>> $ chflags noarch $tmpfile
> >>>> $ rm $tmpfile
> >>>>
> >>>> Currently chflags(2) states:
> >>>>
> >>>>      The SF_IMMUTABLE, SF_APPEND, SF_NOUNLINK, and SF_ARCHIVED flags may only
> >>>>      be set or unset by the super-user.  Attempts to set these flags by non-
> >>>>      super-users are rejected, >>> attempts by non-superusers to clear
> >>>> flags that
> >>>>      are already unset are silently ignored. <<<  These flags may be set at any
> >>>>      time, but normally may only be unset when the system is in single-user
> >>>>      mode.  (See init(8) for details.)
> >>>>
> >>>> So this behavior is already well documented :). The EPERM section
> >>>> should really note SF_ARCHIVED though (whoever added the flag forgot
> >>>> to add that particular item to the ERRORS section).
> >>>
> >>> that's perfectly alright. clearing an unset flag shouldn't cause any error to
> >>> be returned. however in my example arch *does* get set and still trying to
> >>> unset it as normal user doesn't return an error.
> >>>
> >>
> >> It's even more interesting.
> >>
> >> As far as I could parse the code:
> >> - UFS has no special handling for SF_ARCHIVED (I found only it for msdosfs)
> >
> >    _very_ interesting:
> >
> > [/sys]$ grep -r SF_ARCHIVED kern/ fs/ ufs/ | grep -v svn
> > fs/msdosfs/msdosfs_vnops.c:             vap->va_flags |= SF_ARCHIVED;
> > fs/msdosfs/msdosfs_vnops.c:             if (vap->va_flags & ~SF_ARCHIVED)
> > fs/msdosfs/msdosfs_vnops.c:             if (vap->va_flags & SF_ARCHIVED)
> >
> >    The commit that introduced this change probably wasn't doing the
> > right thing: http://svn.freebsd.org/viewvc/base/head/sys/fs/msdosfs/msdosfs_vnops.c?revision=5241&view=markup
> > ; cp(1) probably should have been fixed in lieu of `fixing' msdosfs.
> >
> >> - ufs_setattr() does not handle unsetting SF_ARCHIVED,
> >>  so all what it does is simply return zero.
> >
> >     [EOPNOTSUPP]       The underlying file system does not support file
> >                        flags.
> >
> >    So I would expect for invalid flags to return EOPNOTSUPP.
> >
> > ...
> >
> > $ ~/test_chflags_negative
> > test_chflags_negative: should not get here
> > $ sudo ~/test_chflags_negative
> > test_chflags_negative: should not get here
> >
> > *facepalm*
> >
> >    I think the problem in part is here (sys/stat.h):
> >
> >  *
> >  * Super-user and owner changeable flags.
> >  */
> > #define UF_SETTABLE     0x0000ffff      /* mask of owner changeable flags */
> > #define UF_NODUMP       0x00000001      /* do not dump file */
> > #define UF_IMMUTABLE    0x00000002      /* file may not be changed */
> > #define UF_APPEND       0x00000004      /* writes to file may only append */
> > #define UF_OPAQUE       0x00000008      /* directory is opaque wrt. union */
> > #define UF_NOUNLINK     0x00000010      /* file may not be removed or renamed */
> > /*
> >  * Super-user changeable flags.
> >  */
> > #define SF_SETTABLE     0xffff0000      /* mask of superuser changeable flags */
> > #define SF_ARCHIVED     0x00010000      /* file is archived */
> > #define SF_IMMUTABLE    0x00020000      /* file may not be changed */
> > #define SF_APPEND       0x00040000      /* writes to file may only append */
> > #define SF_NOUNLINK     0x00100000      /* file may not be removed or renamed */
> > #define SF_SNAPSHOT     0x00200000      /* snapshot inode */
> >
> >    Note the *_SETTABLE macros, and the fact that they allow for more
> > functionality than what's currently slotted with the one-hot encoded
> > flags currently available.
> >    SF_ARCHIVED is not present in the other BSDs or Mac OSX either (I
> > did some hunting for a python bug related to chflags a few weeks
> > ago)... and I'm not even sure what this functionality really buys us
> > because it's not well described (but I'd be happy to get an
> > explanation/history lesson).
> >
> >> - /bin/chflags doesn't check the actual flags value from inode after
> >> calling chflags() syscall, and blindly assumes all is well, if chflags()
> >> returns with zero,
> >
> >    Yeah... but ideally tests should be written for this stuff and
> > exercised on all filesystems and exercised whenever code in this
> > particular path is changed, because that would potentially turn into a
> > noticeable performance hit [depending on how it's implemented in
> > chflags(1)]. And lo and behold it already does exist under
> > .../tools/regression/fstest/tests/chflags . I'll audit this once I get
> > back home...
> 
>     For starters, the tests were moved to .../tools/regression/pjdfstest .
>     This fixes the manpage and the negative flags testcase at least. I
> ran the pjdfstest on a UFS2 partition on my test machine and tmpfs,
> and it passed chflags with flying colors. msdosfs unfortunately isn't
> supported yet, but I did some manual testing and everything seemed ok.
> I also need to check and see whether or not pjdfstest is doing the
> right job with negative testcases.
>     I didn't have a ext2/3 or zfs pool to test with, so if someone
> could poke around with those filesystems it would be much appreciated
> :).
>     And finally, here are all of the references in the sourcebase to
> SF_ARCHIVED:
> 
> # /usr/local/bin/svnversion
> 213377M
> # grep -r SF_ARCHIVED /usr/src/ | grep -v svn
> grep: /usr/src/tools/regression/pjdfstest/pjdfstest_5aaec5b222b60945b16daa0e8d61313d/pjdfstest_b4353ca81458e0bfc9ec5be8ff741eb2/usr/src/tools/regression/priv/priv_vfs_chflags.c:	flags
> |= SF_ARCHIVED;
> /usr/src/tools/regression/priv/priv_vfs_chflags.c:	flags |= SF_ARCHIVED;
> /usr/src/tools/regression/priv/priv_vfs_chflags.c:	flags |= SF_ARCHIVED;
> /usr/src/tools/regression/pjdfstest/tests/chflags/00.t:	allflags="UF_NODUMP,UF_IMMUTABLE,UF_APPEND,UF_NOUNLINK,UF_OPAQUE,SF_ARCHIVED,SF_IMMUTABLE,SF_APPEND,SF_NOUNLINK"
> /usr/src/tools/regression/pjdfstest/tests/chflags/00.t:	systemflags="SF_ARCHIVED,SF_IMMUTABLE,SF_APPEND,SF_NOUNLINK"
> Binary file /usr/src/tools/regression/pjdfstest/pjdfstest matches
> /usr/src/tools/regression/pjdfstest/pjdfstest.c:#ifdef SF_ARCHIVED
> /usr/src/tools/regression/pjdfstest/pjdfstest.c:	{ SF_ARCHIVED, "SF_ARCHIVED" },
> : Operation not supported
> grep: warning: /usr/src/sys/modules/tmpfs/@: recursive directory loop
> /usr/src/lib/libc/gen/strtofflags.c:	{ "noarch",		SF_ARCHIVED,	0 },
> /usr/src/lib/libc/gen/strtofflags.c:	{ "noarchived",		SF_ARCHIVED,	0 },
> /usr/src/lib/libc/sys/chflags.2:.It Dv SF_ARCHIVED
> /usr/src/lib/libc/sys/chflags.2:.Dv SF_ARCHIVED
> /usr/src/lib/libc/sys/chflags.2:.Dv SF_ARCHIVED , SF_IMMUTABLE , SF_APPEND ,
> /usr/src/lib/libc/sys/chflags.2:.Dv SF_ARCHIVED , SF_IMMUTABLE , SF_APPEND ,
> /usr/src/lib/libarchive/archive_entry.c:#ifdef SF_ARCHIVED
> /usr/src/lib/libarchive/archive_entry.c:	{
> "noarch",	L"noarch",	SF_ARCHIVED,	0 },
> /usr/src/lib/libarchive/archive_entry.c:	{
> "noarchived",	L"noarchived",       	SF_ARCHIVED,	0 },
> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c:		vap->va_flags |= SF_ARCHIVED;
> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c:		if (vap->va_flags & ~SF_ARCHIVED)
> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c:		if (vap->va_flags & SF_ARCHIVED)
> /usr/src/sys/sys/stat.h:#define	SF_ARCHIVED	0x00010000	/* file is archived */
> /usr/src/sys/sys/stat.h:#define	SF_SETTABLE	(SF_ARCHIVED |
> SF_IMMUTABLE | SF_APPEND | \
> 
>     So it doesn't look like anything's utilizing the functionality,
> other than msdosfs, and all that really does is tweak the following
> attribute:
> 
> #define ATTR_ARCHIVE    0x20            /* file is new or modified */
> 
>     and vice versa. I vaguely remember archive file types in FAT32
> from the Win95 days, but my memory is a bit hazy as to what the
> attribute actually does.

occasionally i get errors during file copies from msdosfs to ufs2. windows
seems to use the arch flag quite extensively actually. i think formating a new
drive automatically markes it as 'archivable' and all new files get their flag
set.
so when copying files from fat32 to ufs i very often get an EPERM error,
because as a non-root user that flag cannot be preserved. doesn't matter
however since the copying suceeds. good thing the chflags operation takes place
after copying the file and not beforehand (i.e. touch, chflags, copy).

also: what about sorting the flags (in the manual page e.g.). should they be
sorted alphabetically or by bitfields?

also: is SF_SNAPSHOT really changeable by the super user? chflags(2) says it's
not.

cheers.
alex

> Thanks,
> -Garrett




-- 
a13x


More information about the freebsd-hackers mailing list