On Wed, Oct 6, 2010 at 3:01 PM, Sergey Kandaurov <pluk...@gmail.com> wrote: > On 6 October 2010 23:38, Alexander Best <arun...@freebsd.org> wrote: >> On Wed Oct 6 10, Garrett Cooper wrote: >>> On Wed, Oct 6, 2010 at 10:35 AM, Alexander Best <arun...@freebsd.org> wrote: >>> > On Wed Oct 6 10, Garrett Cooper wrote: >>> >> On Tue, Oct 5, 2010 at 4:50 PM, Alexander Best <arun...@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
test_chflags_negative.c
Description: Binary data
_______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"
