Date: Sun, 10 Oct 2010 22:53:51 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Alexander Best <arundel@freebsd.org> Cc: freebsd-hackers@freebsd.org, Sergey Kandaurov <pluknet@gmail.com> Subject: Re: issue with unsetting 'arch' flag Message-ID: <AANLkTimrgwr49wjxzQ=pNNu8Ru-fz_wKT2NMzivA_S%2B=@mail.gmail.com> In-Reply-To: <20101007103601.GA17089@freebsd.org> References: <20101005235054.GA45827@freebsd.org> <AANLkTi=sA4GP=B61tbEmG6B0CYcET=dCFMJByoS_5=yi@mail.gmail.com> <20101006173522.GA92402@freebsd.org> <AANLkTi==F4zFmJxqOBzMCk%2Buci6XbvoQBe4mqxHjtbr6@mail.gmail.com> <20101006193827.GA13528@freebsd.org> <AANLkTikYX0vsxZi=J6Asekk-Kd_Y4MyemjDxM5FXARng@mail.gmail.com> <AANLkTimut3obh4VgKVv3PCgicwEKK4f0zg=W2OnSv86s@mail.gmail.com> <AANLkTime33mbPkmudgpTsz-Z-THrovvbjcDtdihRBGBg@mail.gmail.com> <20101007103601.GA17089@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, Oct 7, 2010 at 3:36 AM, Alexander Best <arundel@freebsd.org> wrote: > On Wed Oct =A06 10, Garrett Cooper wrote: >> On Wed, Oct 6, 2010 at 4:03 PM, Garrett Cooper <gcooper@freebsd.org> wro= te: >> > On Wed, Oct 6, 2010 at 3:01 PM, Sergey Kandaurov <pluknet@gmail.com> w= rote: >> >> On 6 October 2010 23:38, Alexander Best <arundel@freebsd.org> wrote: >> >>> On Wed Oct =A06 10, Garrett Cooper wrote: >> >>>> On Wed, Oct 6, 2010 at 10:35 AM, Alexander Best <arundel@freebsd.or= g> wrote: >> >>>> > On Wed Oct =A06 10, Garrett Cooper wrote: >> >>>> >> On Tue, Oct 5, 2010 at 4:50 PM, Alexander Best <arundel@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`. >> >>>> >> > =A0^^non-root =A0 =A0 ^^root =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0^^= non-root >> >>>> >> > >> >>>> >> > chflags claims to have cleared the 'arch' flag (which should b= e impossible as >> >>>> >> > non-root user), but indeed has done nothing. >> >>>> >> > >> >>>> >> > i've tried the same with 'sappnd' and that works as can be exp= ected. >> >>>> >> > >> >>>> >> > The issue was confirmed to exist in HEAD (me), stable/8 (pgoll= ucc1, jpaetzel) >> >>>> >> > and stable/7 (nox). >> >>>> >> > On stable/6 it does NOT exist (jpaetzel). chflags properly fai= ls with EPERM. >> >>>> >> >> >>>> >> =A0 =A0 Fails for me when I call the syscall directly, as I woul= d expect, >> >>>> >> and passes when I'm superuser: >> >>>> >> >> >>>> >> $ ./test_chflags >> >>>> >> (uid, euid) =3D (1000, 1000) >> >>>> >> test_chflags: chflags: Operation not permitted >> >>>> >> test_chflags: lchflags: Operation not permitted >> >>>> >> $ sudo ./test_chflags >> >>>> >> (uid, euid) =3D (0, 0) >> >>>> >> >> >>>> >> =A0 =A0 According to my basic inspection in strtofflags >> >>>> >> (.../lib/libc/gen/strtofflags.c), it works as well. >> >>>> >> =A0 =A0 And last but not least, executing the commands directly = on the CLI work: >> >>>> >> >> >>>> >> $ tmpfile=3D`mktemp /tmp/chflags.XXXXXX` >> >>>> >> $ chflags arch $tmpfile >> >>>> >> chflags: /tmp/chflags.nQm1IL: Operation not permitted >> >>>> >> $ rm $tmpfile >> >>>> >> $ tmpfile=3D`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 n= ot sure >> >>>> > however you understood the problem. probably i didn't explain it = right: >> >>>> > >> >>>> > $ sudo rm -d /tmp/chflags.XXXXXX >> >>>> > $ tmpfile=3D`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=3D`mktemp /tmp/chflags.XXXXXX` >> >>>> $ chflags arch $tmpfile >> >>>> chflags: /tmp/chflags.V2NpXR: Operation not permitted >> >>>> $ chflags noarch $tmpfile >> >>>> $ rm $tmpfile >> >>>> >> >>>> Currently chflags(2) states: >> >>>> >> >>>> =A0 =A0 =A0The SF_IMMUTABLE, SF_APPEND, SF_NOUNLINK, and SF_ARCHIVE= D flags may only >> >>>> =A0 =A0 =A0be set or unset by the super-user. =A0Attempts to set th= ese flags by non- >> >>>> =A0 =A0 =A0super-users are rejected, >>> attempts by non-superusers= to clear >> >>>> flags that >> >>>> =A0 =A0 =A0are already unset are silently ignored. <<< =A0These fla= gs may be set at any >> >>>> =A0 =A0 =A0time, but normally may only be unset when the system is = in single-user >> >>>> =A0 =A0 =A0mode. =A0(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 forgo= t >> >>>> 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 try= ing 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 ms= dosfs) >> > >> > =A0 =A0_very_ interesting: >> > >> > [/sys]$ grep -r SF_ARCHIVED kern/ fs/ ufs/ | grep -v svn >> > fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 vap->va_flags |=3D= SF_ARCHIVED; >> > fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 if (vap->va_flags = & ~SF_ARCHIVED) >> > fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 if (vap->va_flags = & SF_ARCHIVED) >> > >> > =A0 =A0The commit that introduced this change probably wasn't doing th= e >> > right thing: http://svn.freebsd.org/viewvc/base/head/sys/fs/msdosfs/ms= dosfs_vnops.c?revision=3D5241&view=3Dmarkup >> > ; cp(1) probably should have been fixed in lieu of `fixing' msdosfs. >> > >> >> - ufs_setattr() does not handle unsetting SF_ARCHIVED, >> >> =A0so all what it does is simply return zero. >> > >> > =A0 =A0 [EOPNOTSUPP] =A0 =A0 =A0 The underlying file system does not s= upport file >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0flags. >> > >> > =A0 =A0So 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* >> > >> > =A0 =A0I think the problem in part is here (sys/stat.h): >> > >> > =A0* >> > =A0* Super-user and owner changeable flags. >> > =A0*/ >> > #define UF_SETTABLE =A0 =A0 0x0000ffff =A0 =A0 =A0/* mask of owner cha= ngeable flags */ >> > #define UF_NODUMP =A0 =A0 =A0 0x00000001 =A0 =A0 =A0/* do not dump fil= e */ >> > #define UF_IMMUTABLE =A0 =A00x00000002 =A0 =A0 =A0/* file may not be c= hanged */ >> > #define UF_APPEND =A0 =A0 =A0 0x00000004 =A0 =A0 =A0/* writes to file = may only append */ >> > #define UF_OPAQUE =A0 =A0 =A0 0x00000008 =A0 =A0 =A0/* directory is op= aque wrt. union */ >> > #define UF_NOUNLINK =A0 =A0 0x00000010 =A0 =A0 =A0/* file may not be r= emoved or renamed */ >> > /* >> > =A0* Super-user changeable flags. >> > =A0*/ >> > #define SF_SETTABLE =A0 =A0 0xffff0000 =A0 =A0 =A0/* mask of superuser= changeable flags */ >> > #define SF_ARCHIVED =A0 =A0 0x00010000 =A0 =A0 =A0/* file is archived = */ >> > #define SF_IMMUTABLE =A0 =A00x00020000 =A0 =A0 =A0/* file may not be c= hanged */ >> > #define SF_APPEND =A0 =A0 =A0 0x00040000 =A0 =A0 =A0/* writes to file = may only append */ >> > #define SF_NOUNLINK =A0 =A0 0x00100000 =A0 =A0 =A0/* file may not be r= emoved or renamed */ >> > #define SF_SNAPSHOT =A0 =A0 0x00200000 =A0 =A0 =A0/* snapshot inode */ >> > >> > =A0 =A0Note the *_SETTABLE macros, and the fact that they allow for mo= re >> > functionality than what's currently slotted with the one-hot encoded >> > flags currently available. >> > =A0 =A0SF_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 chflag= s() >> >> returns with zero, >> > >> > =A0 =A0Yeah... 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... >> >> =A0 =A0 For starters, the tests were moved to .../tools/regression/pjdfs= test . >> =A0 =A0 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. >> =A0 =A0 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 >> :). >> =A0 =A0 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_5aaec5b222b60945b16d= aa0e8d61313d/pjdfstest_b4353ca81458e0bfc9ec5be8ff741eb2/usr/src/tools/regre= ssion/priv/priv_vfs_chflags.c: =A0 =A0 flags >> |=3D SF_ARCHIVED; >> /usr/src/tools/regression/priv/priv_vfs_chflags.c: =A0 =A0flags |=3D SF_= ARCHIVED; >> /usr/src/tools/regression/priv/priv_vfs_chflags.c: =A0 =A0flags |=3D SF_= ARCHIVED; >> /usr/src/tools/regression/pjdfstest/tests/chflags/00.t: =A0 =A0 =A0 allf= lags=3D"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: =A0 =A0 =A0 syst= emflags=3D"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: =A0 =A0 =A0{ SF_ARCHIVE= D, "SF_ARCHIVED" }, >> : Operation not supported >> grep: warning: /usr/src/sys/modules/tmpfs/@: recursive directory loop >> /usr/src/lib/libc/gen/strtofflags.c: =A0{ "noarch", =A0 =A0 =A0 =A0 =A0 = =A0 SF_ARCHIVED, =A0 =A00 }, >> /usr/src/lib/libc/gen/strtofflags.c: =A0{ "noarchived", =A0 =A0 =A0 =A0 = SF_ARCHIVED, =A0 =A00 }, >> /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_APPE= ND , >> /usr/src/lib/libc/sys/chflags.2:.Dv SF_ARCHIVED , SF_IMMUTABLE , SF_APPE= ND , >> /usr/src/lib/libarchive/archive_entry.c:#ifdef SF_ARCHIVED >> /usr/src/lib/libarchive/archive_entry.c: =A0 =A0 =A0{ >> "noarch", =A0 =A0 L"noarch", =A0 =A0 =A0SF_ARCHIVED, =A0 =A00 }, >> /usr/src/lib/libarchive/archive_entry.c: =A0 =A0 =A0{ >> "noarchived", L"noarchived", =A0 =A0 =A0 =A0 =A0SF_ARCHIVED, =A0 =A00 }, >> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 =A0vap-= >va_flags |=3D SF_ARCHIVED; >> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 =A0if (= vap->va_flags & ~SF_ARCHIVED) >> /usr/src/sys/fs/msdosfs/msdosfs_vnops.c: =A0 =A0 =A0 =A0 =A0 =A0 =A0if (= vap->va_flags & SF_ARCHIVED) >> /usr/src/sys/sys/stat.h:#define =A0 =A0 =A0 SF_ARCHIVED =A0 =A0 0x000100= 00 =A0 =A0 =A0/* file is archived */ >> /usr/src/sys/sys/stat.h:#define =A0 =A0 =A0 SF_SETTABLE =A0 =A0 (SF_ARCH= IVED | >> SF_IMMUTABLE | SF_APPEND | \ >> >> =A0 =A0 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 =A0 =A00x20 =A0 =A0 =A0 =A0 =A0 =A0/* file is new o= r modified */ >> >> =A0 =A0 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. window= s > 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. According to sbruno@: "It gets set when a file is written to. That's it. A backup program can backup files then clear the bit; later backups can use the state of the bit to determine if the file needs backing up again. And it dates from MS-DOS 2.x, and possibly even 1.x. During those times you couldn't exactly trust timestamps on files meant anything for backup purposes, as PCs didn't have a battery backed RTC as standard. http://en.wikipedia.org/wiki/Archive_bit" So it exists purely for msdosfs for that historic reason. > 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). Yeah... > also: what about sorting the flags (in the manual page e.g.). should they= be > sorted alphabetically or by bitfields? They're currently sorted by value (including the patch I attached earlier). > also: is SF_SNAPSHOT really changeable by the super user? chflags(2) says= it's > not. >From what I've seen, not directly. It's used widely in querying filesystems though. Thanks, -Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimrgwr49wjxzQ=pNNu8Ru-fz_wKT2NMzivA_S%2B=>