Date: Wed, 6 Oct 2010 22:37:43 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Garrett Cooper <gcooper@freebsd.org> Cc: Alexander Best <arundel@freebsd.org>, Sergey Kandaurov <pluknet@gmail.com>, freebsd-hackers@freebsd.org Subject: Re: issue with unsetting 'arch' flag Message-ID: <AANLkTime33mbPkmudgpTsz-Z-THrovvbjcDtdihRBGBg@mail.gmail.com> In-Reply-To: <AANLkTimut3obh4VgKVv3PCgicwEKK4f0zg=W2OnSv86s@mail.gmail.com> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
--0050450172391002ee0492004b28 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Wed, Oct 6, 2010 at 4:03 PM, Garrett Cooper <gcooper@freebsd.org> wrote: > On Wed, Oct 6, 2010 at 3:01 PM, Sergey Kandaurov <pluknet@gmail.com> wrot= e: >> 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.org> = 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 lon= g 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 be i= mpossible as >>>> >> > non-root user), but indeed has done nothing. >>>> >> > >>>> >> > i've tried the same with 'sappnd' and that works as can be expect= ed. >>>> >> > >>>> >> > The issue was confirmed to exist in HEAD (me), stable/8 (pgollucc= 1, jpaetzel) >>>> >> > and stable/7 (nox). >>>> >> > On stable/6 it does NOT exist (jpaetzel). chflags properly fails = with EPERM. >>>> >> >>>> >> =A0 =A0 Fails for me when I call the syscall directly, as I would e= xpect, >>>> >> 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 not = sure >>>> > however you understood the problem. probably i didn't explain it rig= ht: >>>> > >>>> > $ 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, bu= t 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_ARCHIVED f= lags may only >>>> =A0 =A0 =A0be set or unset by the super-user. =A0Attempts to set these= 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 flags = 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 forgot >>>> to add that particular item to the ERRORS section). >>> >>> that's perfectly alright. clearing an unset flag shouldn't cause any er= ror 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 msdos= fs) > > =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 & S= F_ARCHIVED) > > =A0 =A0The commit that introduced this change probably wasn't doing the > right thing: http://svn.freebsd.org/viewvc/base/head/sys/fs/msdosfs/msdos= fs_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 supp= ort 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 change= able flags */ > #define UF_NODUMP =A0 =A0 =A0 0x00000001 =A0 =A0 =A0/* do not dump file *= / > #define UF_IMMUTABLE =A0 =A00x00000002 =A0 =A0 =A0/* file may not be chan= ged */ > #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 opaqu= e wrt. union */ > #define UF_NOUNLINK =A0 =A0 0x00000010 =A0 =A0 =A0/* file may not be remo= ved or renamed */ > /* > =A0* Super-user changeable flags. > =A0*/ > #define SF_SETTABLE =A0 =A0 0xffff0000 =A0 =A0 =A0/* mask of superuser ch= angeable 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 chan= ged */ > #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 remo= ved 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 more > 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 chflags() >> 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... 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_5aaec5b222b60945b16daa0= e8d61313d/pjdfstest_b4353ca81458e0bfc9ec5be8ff741eb2/usr/src/tools/regressi= on/priv/priv_vfs_chflags.c: flags |=3D SF_ARCHIVED; /usr/src/tools/regression/priv/priv_vfs_chflags.c: flags |=3D SF_ARCHIVED; /usr/src/tools/regression/priv/priv_vfs_chflags.c: flags |=3D SF_ARCHIVED; /usr/src/tools/regression/pjdfstest/tests/chflags/00.t: allflags=3D"UF_NODU= MP,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=3D"SF_A= RCHIVED,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_ARCHIVE= D" }, : 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 |=3D 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. Thanks, -Garrett --0050450172391002ee0492004b28 Content-Type: application/octet-stream; name="note-EPERM-SF_ARCHIVED-requirement.diff" Content-Disposition: attachment; filename="note-EPERM-SF_ARCHIVED-requirement.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gez5wqb21 SW5kZXg6IGxpYi9saWJjL3N5cy9jaGZsYWdzLjIKPT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gbGliL2xpYmMvc3lz L2NoZmxhZ3MuMgkocmV2aXNpb24gMjEzMzc3KQorKysgbGliL2xpYmMvc3lzL2NoZmxhZ3MuMgko d29ya2luZyBjb3B5KQpAQCAtMTQ4LDE0ICsxNDgsMTQgQEAKIHRoZSBlZmZlY3RpdmUgdXNlciBJ RCBpcyBub3QgdGhlIHN1cGVyLXVzZXIuCiAuSXQgQnEgRXIgRVBFUk0KIE9uZSBvZgotLkR2IFNG X0lNTVVUQUJMRSAsIFNGX0FQUEVORCAsCisuRHYgU0ZfQVJDSElWRUQgLCBTRl9JTU1VVEFCTEUg LCBTRl9BUFBFTkQgLAogb3IKIC5EdiBTRl9OT1VOTElOSwogaXMgc2V0IGFuZCB0aGUgdXNlciBp cyBlaXRoZXIgbm90IHRoZSBzdXBlci11c2VyIG9yCiBzZWN1cmVsZXZlbCBpcyBncmVhdGVyIHRo YW4gMC4KIC5JdCBCcSBFciBFUEVSTQogQSBub24tc3VwZXItdXNlciB0cmllcyB0byBzZXQgb25l IG9mCi0uRHYgU0ZfSU1NVVRBQkxFICwgU0ZfQVBQRU5EICwKKy5EdiBTRl9BUkNISVZFRCAsIFNG X0lNTVVUQUJMRSAsIFNGX0FQUEVORCAsCiBvcgogLkR2IFNGX05PVU5MSU5LIC4KIC5JdCBCcSBF ciBFUEVSTQo= --0050450172391002ee0492004b28 Content-Type: application/octet-stream; name="fail-chflags-with-bad-flags.diff" Content-Disposition: attachment; filename="fail-chflags-with-bad-flags.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gez6b9ry2 SW5kZXg6IHN5cy9rZXJuL3Zmc19zeXNjYWxscy5jCj09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIHN5cy9rZXJuL3Zm c19zeXNjYWxscy5jCShyZXZpc2lvbiAyMTMzNzcpCisrKyBzeXMva2Vybi92ZnNfc3lzY2FsbHMu Ywkod29ya2luZyBjb3B5KQpAQCAtMjcxMiw2ICsyNzEyLDggQEAKIAlpbnQgdmZzbG9ja2VkOwog CiAJQVVESVRfQVJHX0ZGTEFHUyh1YXAtPmZsYWdzKTsKKwlpZiAoKHVhcC0+ZmxhZ3MgJiAoU0Zf U0VUVEFCTEUgfCBVRl9TRVRUQUJMRSkpICE9IHVhcC0+ZmxhZ3MpCisJCXJldHVybiAoRU9QTk9U U1VQUCk7CiAJTkRJTklUKCZuZCwgTE9PS1VQLCBGT0xMT1cgfCBNUFNBRkUgfCBBVURJVFZOT0RF MSwgVUlPX1VTRVJTUEFDRSwKIAkgICAgdWFwLT5wYXRoLCB0ZCk7CiAJaWYgKChlcnJvciA9IG5h bWVpKCZuZCkpICE9IDApCkluZGV4OiBzeXMvc3lzL3N0YXQuaAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBzeXMv c3lzL3N0YXQuaAkocmV2aXNpb24gMjEzMzc3KQorKysgc3lzL3N5cy9zdGF0LmgJKHdvcmtpbmcg Y29weSkKQEAgLTI2MSwyMiArMjYxLDI4IEBACiAgKgogICogU3VwZXItdXNlciBhbmQgb3duZXIg Y2hhbmdlYWJsZSBmbGFncy4KICAqLwotI2RlZmluZQlVRl9TRVRUQUJMRQkweDAwMDBmZmZmCS8q IG1hc2sgb2Ygb3duZXIgY2hhbmdlYWJsZSBmbGFncyAqLwogI2RlZmluZQlVRl9OT0RVTVAJMHgw MDAwMDAwMQkvKiBkbyBub3QgZHVtcCBmaWxlICovCiAjZGVmaW5lCVVGX0lNTVVUQUJMRQkweDAw MDAwMDAyCS8qIGZpbGUgbWF5IG5vdCBiZSBjaGFuZ2VkICovCiAjZGVmaW5lCVVGX0FQUEVORAkw eDAwMDAwMDA0CS8qIHdyaXRlcyB0byBmaWxlIG1heSBvbmx5IGFwcGVuZCAqLwogI2RlZmluZSBV Rl9PUEFRVUUJMHgwMDAwMDAwOAkvKiBkaXJlY3RvcnkgaXMgb3BhcXVlIHdydC4gdW5pb24gKi8K ICNkZWZpbmUgVUZfTk9VTkxJTksJMHgwMDAwMDAxMAkvKiBmaWxlIG1heSBub3QgYmUgcmVtb3Zl ZCBvciByZW5hbWVkICovCisKKwkJCQkJLyogbWFzayBvZiBzdXBlcnVzZXIgY2hhbmdlYWJsZSBm bGFncyAqLworI2RlZmluZQlVRl9TRVRUQUJMRQkoVUZfTk9EVU1QIHwgVUZfSU1NVVRBQkxFIHwg VUZfQVBQRU5EIHwgVUZfT1BBUVVFIHwgXAorCQkJIFVGX05PVU5MSU5LKQogLyoKICAqIFN1cGVy LXVzZXIgY2hhbmdlYWJsZSBmbGFncy4KICAqLwotI2RlZmluZQlTRl9TRVRUQUJMRQkweGZmZmYw MDAwCS8qIG1hc2sgb2Ygc3VwZXJ1c2VyIGNoYW5nZWFibGUgZmxhZ3MgKi8KICNkZWZpbmUJU0Zf QVJDSElWRUQJMHgwMDAxMDAwMAkvKiBmaWxlIGlzIGFyY2hpdmVkICovCiAjZGVmaW5lCVNGX0lN TVVUQUJMRQkweDAwMDIwMDAwCS8qIGZpbGUgbWF5IG5vdCBiZSBjaGFuZ2VkICovCiAjZGVmaW5l CVNGX0FQUEVORAkweDAwMDQwMDAwCS8qIHdyaXRlcyB0byBmaWxlIG1heSBvbmx5IGFwcGVuZCAq LwogI2RlZmluZQlTRl9OT1VOTElOSwkweDAwMTAwMDAwCS8qIGZpbGUgbWF5IG5vdCBiZSByZW1v dmVkIG9yIHJlbmFtZWQgKi8KICNkZWZpbmUJU0ZfU05BUFNIT1QJMHgwMDIwMDAwMAkvKiBzbmFw c2hvdCBpbm9kZSAqLwogCisJCQkJCS8qIG1hc2sgb2Ygc3VwZXJ1c2VyIGNoYW5nZWFibGUgZmxh Z3MgKi8KKyNkZWZpbmUJU0ZfU0VUVEFCTEUJKFNGX0FSQ0hJVkVEIHwgU0ZfSU1NVVRBQkxFIHwg U0ZfQVBQRU5EIHwgXAorCQkJIFNGX05PVU5MSU5LIHwgU0ZfU05BUFNIT1QpCisKICNpZmRlZiBf S0VSTkVMCiAvKgogICogU2hvcnRoYW5kIGFiYnJldmlhdGlvbnMgb2YgYWJvdmUuCg== --0050450172391002ee0492004b28--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTime33mbPkmudgpTsz-Z-THrovvbjcDtdihRBGBg>