issue with unsetting 'arch' flag

Garrett Cooper gcooper at FreeBSD.org
Wed Oct 6 23:03:02 UTC 2010 

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...

Thanks,
-Garrett
-------------- next part --------------
A non-text attachment was scrubbed...
Name: test_chflags_negative.c
Type: application/octet-stream
Size: 181 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-hackers/attachments/20101006/bbff5561/test_chflags_negative.obj

More information about the freebsd-hackers mailing list