From owner-freebsd-fs@FreeBSD.ORG Sat Mar 9 00:12:53 2013 Return-Path: Delivered-To: fs@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 82CC47D7; Sat, 9 Mar 2013 00:12:53 +0000 (UTC) (envelope-from rmacklem@uoguelph.ca) Received: from esa-annu.net.uoguelph.ca (esa-annu.mail.uoguelph.ca [131.104.91.36]) by mx1.freebsd.org (Postfix) with ESMTP id C7BC715D; Sat, 9 Mar 2013 00:12:52 +0000 (UTC) X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AqIEANx9OlGDaFvO/2dsb2JhbAA9BoglvDaBc3SCLAEBAQMBAQEBIAQnIAsFFhgCAg0ZAikBCRgBDQYIBwQBHASHbAYMqWGSNYEjjDgMcQEzB4ItgRMDiHGKHYEHgj6BHo9UgyhPgQU1 X-IronPort-AV: E=Sophos;i="4.84,810,1355115600"; d="scan'208";a="17831967" Received: from erie.cs.uoguelph.ca (HELO zcs3.mail.uoguelph.ca) ([131.104.91.206]) by esa-annu.net.uoguelph.ca with ESMTP; 08 Mar 2013 19:11:44 -0500 Received: from zcs3.mail.uoguelph.ca (localhost.localdomain [127.0.0.1]) by zcs3.mail.uoguelph.ca (Postfix) with ESMTP id 7CBF8B402B; Fri, 8 Mar 2013 19:11:44 -0500 (EST) Date: Fri, 8 Mar 2013 19:11:44 -0500 (EST) From: Rick Macklem To: "Kenneth D. Merry" Message-ID: <37540270.3721223.1362787904494.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130308232155.GA47062@nargothrond.kdm.org> Subject: Re: patches to add new stat(2) file flags MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Originating-IP: [172.17.91.202] X-Mailer: Zimbra 6.0.10_GA_2692 (ZimbraWebClient - FF3.0 (Win)/6.0.10_GA_2692) Cc: arch@FreeBSD.org, fs@FreeBSD.org X-BeenThere: freebsd-fs@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Filesystems List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 09 Mar 2013 00:12:53 -0000 Kenneth D. Merry wrote: > On Fri, Mar 08, 2013 at 00:37:15 +1100, Bruce Evans wrote: > > On Wed, 6 Mar 2013, Kenneth D. Merry wrote: > > > > >I have attached diffs against head for some additional stat(2) file > > >flags. > > > > > >The primary purpose of these flags is to improve compatibility with > > >CIFS, > > >both from the client and the server side. > > >... > > > > I missed looking at the diffs in my previous reply. > > > > % --- //depot/users/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.86 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/bin/chflags/chflags.1 > > 2013-03-06 14:47:25.987128763 -0700 > > % @@ -117,6 +117,16 @@ > > % set the user immutable flag (owner or super-user only) > > % .It Cm uunlnk , uunlink > > % set the user undeletable flag (owner or super-user only) > > % +.It Cm system , usystem > > % +set the Windows system flag (owner or super-user only) > > > > This begins unsorting of the list. > > Fixed. > > > It's not just a Windows flag, since it also works in DOS. > > Fixed. > > > "Owner or" is too strict for msdosfs, since files can only have a > > single owner so it is controlling access using groups is needed. I > > use owner root and group msdosfs for msdosfs mounts. This works for > > normal operations like open/read/write, but fails for most > > attributes > > including file flags. msdosfs doesn't support many attributes but > > this change is supposed to add support for 3 new file flags so it > > would > > be good if it didn't restrict the support to root. > > I wasn't trying to change the existing security model for msdosfs, but > if > you've got a suggested patch to fix it I can add that in. > > > % +.It Cm sparse , usparse > > % +set the sparse file attribute (owner or super-user only) > > % +.It Cm offline , uoffline > > % +set the offline file attribute (owner or super-user only) > > % +.It Cm reparse , ureparse > > % +set the Windows reparse point file attribute (owner or super-user > > only) > > % +.It Cm hidden , uhidden > > % +set the hidden file attribute (owner or super-user only) > > > > The additions at the end are also internally unsorted. > > Fixed. > > > Previously only "opaque" and "nodump" were unsorted. They are UF > > flags > > sorted with the SF flags, and "no" in "nodump" is not ignored for > > the > > purposes of sorting. > > > > Not having "u" in the old and new UF flag names messes up the sort > > order > > (unless you add futher confusion by ignoring "u" for the purposes of > > sorting) and makes it harder to add SF variants of the flags. > > They're now sorted in alphabetical order. > > > % .El > > % .Pp > > % Putting the letters > > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-04 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.178 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/gen/strtofflags.c > > 2013-03-06 15:09:32.842437917 -0700 > > % @@ -68,7 +68,17 @@ > > % { "nodump", 1, UF_NODUMP }, > > % { "noopaque", 0, UF_OPAQUE }, > > % { "nouunlnk", 0, UF_NOUNLINK }, > > % - { "nouunlink", 0, UF_NOUNLINK } > > % + { "nouunlink", 0, UF_NOUNLINK }, > > % + { "nosystem", 0, UF_SYSTEM, }, > > % + { "nousystem", 0, UF_SYSTEM, }, > > % + { "nosparse", 0, UF_SPARSE, }, > > % + { "nousparse", 0, UF_SPARSE, }, > > % + { "nooffline", 0, UF_OFFLINE, }, > > % + { "nouoffline", 0, UF_OFFLINE, }, > > % + { "noreparse", 0, UF_REPARSE, }, > > % + { "noureparse", 0, UF_REPARSE, }, > > % + { "nohidden", 0, UF_HIDDEN, }, > > % + { "nouhidden", 0, UF_HIDDEN, } > > > > This is totally disordered too. > > > > The old table was sorted except for "nosnapshot". Another bug is > > that > > "nosnapshot" is supported here (so chflags(1) can show it), but is > > not > > documented in chflags(1). > > Actually, I think the table was previously sorted by the stat(2) flag > name, > and UF_NOUNLINK appears to be the only one that was out of place. I > have > fixed that and my additions. > > > % }; > > % #define nmappings (sizeof(mapping) / sizeof(mapping[0])) > > % > > % --- //depot/users/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.257 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/lib/libc/sys/chflags.2 > > 2013-03-06 14:49:39.357125717 -0700 > > % @@ -75,16 +75,49 @@ > > % Do not dump the file. > > % .It Dv UF_IMMUTABLE > > % The file may not be changed. > > % +Filesystems may use this flag to maintain compatibility with the > > Windows > > and > > % +CIFS FILE_ATTRIBUTE_READONLY attribute. > > % .It Dv UF_APPEND > > % The file may only be appended to. > > > > This was already totally disordered. It even has UF's before SF's, > > although > > SF's sort before UF's both lexically and logically, though not in > > sys/stat.h > > or in bits. > > > > The disorder here was apparently copied from the implementation > > (sys/stat.h, which is in the order of the bits, which is historical > > for binary compatibility) and not cleaned up like chflags(1) and > > strttoflags(3). > > Fixed. > > > % .It Dv UF_NOUNLINK > > % The file may not be renamed or deleted. > > % .It Dv UF_OPAQUE > > % The directory is opaque when viewed through a union stack. > > % +.It Dv UF_SYSTEM > > % +The file has the Windows and CIFS FILE_ATTRIBUTE_SYSTEM > > attribute. > > % +Filesystems in FreeBSD may store and display this flag, but do > > not > > provide > > % +any special handling when it is set. > > > > More disordered than before... > > > > % +.It Dv UF_HIDDEN > > % +The file may be hidden from directory listings at the > > application's > > % +discretion. > > % +The file has the Windows FILE_ATTRIBUTE_HIDDEN attribute. > > > > Not just Windows... > > Fixed. > > > % .It Dv SF_ARCHIVED > > % -The file may be archived. > > % +The file has been archived. > > % +This flag means the opposite of the Windows and CIFS > > FILE_ATTRIBUTE_ARCHIVE > > % +attribute. > > % +That attribute means that the file should be archived, whereas > > % +.Dv SF_ARCHIVED > > % +means that the file has been archived. > > > > WinXP uses the poor wording "avialable for archiving". This is > > better. > > > > % +Filesystems in FreeBSD may or may not have special handling for > > this > > flag. > > % +For instance, ZFS tracks changes to files and will clear this bit > > when a > > % +file is updated. > > % +UFS only stores the flag, and relies on the application to change > > it when > > % +needed. > > > > I think that is useless, since changing it is needed whenever the > > file > > changes, and applications can do that (short of running as daemons > > and > > watching for changes). > > Do you mean applications can't do that or can? > > > WinXP seems to not set the flag when attributes change. I think that > > is > > a bug, and FreeBSD msdosfs does set it when attributes are changed > > (except > > for the SF_ARCHIVED attribute itself, and atimes when the atimes are > > set > > automatically). The FreeBSD behaviour is like setting the ctime for > > any > > change and is bug for bug compatible in doing this even for null > > changes. > > FreeBSD doesn't have many attributes to set, but you just added some > > more. > > I think this should be controlled by a mount option so that bug for > > bug > > compatibility with WinDOS is possible. > > > > % .It Dv SF_IMMUTABLE > > % The file may not be changed. > > % +This flag also indicates that the Windows ans CIFS > > FILE_ATTRIBUTE_READONLY > > % +attribute is set. > > > > s/ans/and/ > > > > You only mentioned UF_IMMUTABLE in your general description. Mapping > > READONLY to both gives even more confusing semantics, and mapping it > > to > > SF_IMMUTABLE seems too strict since for example it prevents using > > FreeBSD > > to manage the flag at high securelevels. > > I agree. That man page change was left over from an earlier version of > the > code. I took it out. > > > % --- > > //depot/users/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.370 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/fs/msdosfs/msdosfs_vnops.c > > 2013-03-06 14:49:47.179130318 -0700 > > % @@ -345,8 +345,17 @@ > > % vap->va_birthtime.tv_nsec = 0; > > % } > > % vap->va_flags = 0; > > % + /* > > % + * The DOS Archive attribute means that a file needs to be > > % + * archived. The BSD SF_ARCHIVED attribute means that a file has > > % + * been archived. Thus the inversion here. > > % + */ > > > > No need to document it again. It goes without saying that ARCHIVE > > != ARCHIVED. > > I disagree. It wasn't immediately obvious to me that SF_ARCHIVED was > generally used as the inverse of the DOS Archived bit until I started > digging into this. If this helps anyone figure that out more quickly, > it's > useful. > > > % if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) > > % vap->va_flags |= SF_ARCHIVED; > > % + if (dep->de_Attributes & ATTR_HIDDEN) > > % + vap->va_flags |= UF_HIDDEN; > > % + if (dep->de_Attributes & ATTR_SYSTEM) > > % + vap->va_flags |= UF_SYSTEM; > > % vap->va_gen = 0; > > % vap->va_blocksize = pmp->pm_bpcluster; > > % vap->va_bytes = > > % @@ -420,12 +429,21 @@ > > % if (error) > > % return (error); > > % } > > > > The permissions check before this is delicate and was broken and is > > more broken now. It is still short-circuit to handle setting the > > single flag that used to be supported, and is slightly broken for > > that: > > - unprivileged user asks to set ARCHIVE by passing !SF_ARCHIVED. We > > allow that, although this may toggle the flag and normal semantics > > for SF flags is to not allow toggling. > > - unprivileged user asks to clear ARCHIVE by passing SF_ARCHIVED. We > > don't allow that. But we should allow preserving ARCHIVE if it is > > already clear. > > The bug wasn't very important when only 1 flag was supported. Now it > > prevents unprivileged users managing the new UF flags if ARCHIVE is > > clear. Fortunately, this is the unusual case. Anyway, unprivileged > > users can set ARCHIVE by doing some other operation. Even the > > chflags() > > operation should set ARCHIVE and thus allow further chflags()'s that > > now > > keep ARCHIVE set. Except it is very confusing if a chflags() asks > > for > > ARCHIVE to be clear. This request might be just to try to preserve > > the current setting and not want it if other things are changed, or > > it might be to purposely clear it. Changing it from set to clear > > should > > still be privileged. > > I changed it to allow setting or clearing SF_ARCHIVED. Now I can set > or > clear the flag as non-root: > > [root@doc-sd /testpool]# mount_msdosfs -u operator -g operator > /dev/md0 /mnt > [root@doc-sd /testpool]# cd /mnt > [root@doc-sd /mnt]# ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo > [root@doc-sd /mnt]# su operator > [operator@doc-sd /mnt]$ chflags arch foo > [operator@doc-sd /mnt]$ ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator arch 0 Mar 8 16:54 foo > [operator@doc-sd /mnt]$ chflags 0 foo > [operator@doc-sd /mnt]$ ls -lao > total 20 > drwxr-xr-x 1 operator operator arch 16384 Jan 1 1980 . > drwxr-xr-x 27 root wheel - 1024 Mar 8 17:05 .. > -rwxr-xr-x 1 operator operator - 0 Mar 8 16:54 foo > > > See the more complicated permissions check in ffs. It would be > > safest > > to duplicate most of it, to get different permissions checking for > > the > > SF and UF flags. Then decide if we want to keep allowing setting > > ARCHIVE without privilege. > > I think we should allow getting and setting SF_ARCHIVED without > special > privileges. Given how it is generally used, I don't think it should be > restricted to the super-user. > > Can you provide some code demonstrating how the permissions code > should > be changed in msdosfs? I don't know that much about that sort of > thing, > so I'll probably spend an inordinate amount of time stumbling > through it. > > > % - if (vap->va_flags & ~SF_ARCHIVED) > > % + if (vap->va_flags & ~(SF_ARCHIVED|UF_HIDDEN|UF_SYSTEM)) > > > > Style bugs (missing spaces around binary operator '|'). These style > > bugs are common in atribute handling for other fs's but were not > > here. > > Fixed. > > > % return EOPNOTSUPP; > > % if (vap->va_flags & SF_ARCHIVED) > > % dep->de_Attributes &= ~ATTR_ARCHIVE; > > % else if (!(dep->de_Attributes & ATTR_DIRECTORY)) > > % dep->de_Attributes |= ATTR_ARCHIVE; > > > > The comment before this says that we ignore attmps to set > > ATTR_ARCHIVED > > for directories. However, it is out of date. WinXP allows setting it > > and all the new flags for directories, and so do we. > > Do you mean we allow setting it in UFS, or where? Obviously the code > above > won't set it on a directory. > > > % + if (vap->va_flags & UF_HIDDEN) > > % + dep->de_Attributes |= ATTR_HIDDEN; > > % + else > > % + dep->de_Attributes &= ~ATTR_HIDDEN; > > % + if (vap->va_flags & UF_SYSTEM) > > % + dep->de_Attributes |= ATTR_SYSTEM; > > % + else > > % + dep->de_Attributes &= ~ATTR_SYSTEM; > > > > I thought you were adding ATTR_READONLY -> UF_IMMUTABLE here. > > No, I said in the commit message that it could be extended to map > ATTR_READONLY to UF_IMMUTABLE, not that I actually did that. > > I didn't make the change because of the existing code to map readonly > to > standard Unix permissions. > > > The WinXP attrib command (at least on a FAT32 fs) doesn't allow > > setting > > or clearing ARCHIVE (even if it is already set or clear) if any of > > HIDDEN, READONLY or SYSTEM is already set and remains set after the > > command. Thus the HRS attributes act a bit like immutable flags, but > > subtly differently. (ffs has the quite different and worse behaviour > > of allowing chflags() irrespective of immutable flags being set > > before > > or after, provided there is enough privilege to change the immutable > > flags.) Anyway, they should all give some aspects of immutability. > > We could do that for msdosfs, but why add more things for the user to > trip > over given how the filesystem is typically used? Most people probably > use it for USB thumb drives these days. Or perahps on a dual boot > system > to access their Windows partition. > > > % + > > > > Style bug (extra blank line). > > Fixed. > > > % dep->de_flag |= DE_MODIFIED; > > % } > > % > > % --- //depot/users/kenm/FreeBSD-test3/sys/sys/stat.h 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.563 2013-03-06 16:42:43.000000000 -0700 > > % +++ /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/sys/stat.h > > 2013-03-06 > > 15:05:45.936126193 -0700 > > % @@ -268,6 +268,22 @@ > > % #define UF_OPAQUE 0x00000008 /* directory is opaque wrt. union */ > > % #define UF_NOUNLINK 0x00000010 /* file may not be removed or > > renamed */ > > > > Old style bugs: inconsistent space instead of tab after #define for > > the 2 > > newer definitions. > > Fixed. > > > % /* > > % + * These two bits are defined in MacOS X. They are not currently > > used in > > % + * FreeBSD. > > % + */ > > % +#if 0 > > % +#define UF_COMPRESSED 0x00000020 /* file is compressed */ > > % +#define UF_TRACKED 0x00000040 /* renames and deletes are > > tracked */ > > % +#endif > > % + > > % +#define UF_SYSTEM 0x00000080 /* Windows system file bit */ > > % +#define UF_SPARSE 0x00000100 /* sparse file */ > > % +#define UF_OFFLINE 0x00000200 /* file is offline */ > > % +#define UF_REPARSE 0x00000400 /* Windows reparse point file bit > > */ > > % +/* This is the same as the MacOS X definition of UF_HIDDEN */ > > % +#define UF_HIDDEN 0x00008000 /* file is hidden */ > > > > New style bugs: random spaces/tabs after #define. 1 main comment not > > punctuated. > > Fixed. > > > % + > > % +/* > > % * Super-user changeable flags. > > % */ > > % #define SF_SETTABLE 0xffff0000 /* mask of superuser > > changeable flags */ > > % --- //depot/users/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-04 > > 17:51:12.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-04 17:51:12.000000000 -0700 > > % --- /tmp/tmp.49594.581 2013-03-06 16:42:43.000000000 -0700 > > % +++ > > /usr/home/kenm/perforce4/kenm/FreeBSD-test3/sys/ufs/ufs/ufs_vnops.c > > 2013-03-06 15:06:27.004132409 -0700 > > % @@ -529,8 +529,9 @@ > > % } > > % if (vap->va_flags != VNOVAL) { > > % if ((vap->va_flags & ~(UF_NODUMP | UF_IMMUTABLE | UF_APPEND | > > % - UF_OPAQUE | UF_NOUNLINK | SF_ARCHIVED | SF_IMMUTABLE | > > % - SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) != 0) > > % + UF_OPAQUE | UF_NOUNLINK | UF_HIDDEN | UF_SYSTEM | > > % + UF_SPARSE | UF_OFFLINE | UF_REPARSE | SF_ARCHIVED | > > % + SF_IMMUTABLE | SF_APPEND | SF_NOUNLINK | SF_SNAPSHOT)) > > != 0) > > % return (EOPNOTSUPP); > > % if (vp->v_mount->mnt_flag & MNT_RDONLY) > > % return (EROFS); > > > > The random order is harder to read with more flags. > > Fixed. > > > I think unsupported flags should just be unsupported -- most or all > > of the > > new flags and SF_ARCHIVED too. Most need system support to work. > > Without > > system support you can only copy them from other fs's and then lose > > them > > when utilities like tar don't unsderstand them. > > The primary reason I added these flags to UFS is to support CIFS > servers. > I have modifications to Likewise to support storing DOS/Windows/CIFS > attributes as file flags, and we could do the same for Samba. > > My eventual intent is to also add support to our NFS code to get/set > the > attributes that are allowed in the NFS 4.1 standard (hidden, system > and > archive are the ones I could find). So in this case, the filesystem > role > for some of these really is just to store it for someone else. > Just fyi, they are supported by NFSv4.0 as well, so there is no need to wait for the NFSv4.1 server code (which I have just started to work on). If you'd like, I can easily do this in April (or later), if/when your patch is committed. Have fun with it, rick > That is the way that ZFS handles attributes like hidden and system. It > doesn't do anything with them, they're just used by the Solaris CIFS > server. > > Ken > -- > Kenneth Merry > ken@FreeBSD.ORG > > _______________________________________________ > freebsd-arch@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-arch > To unsubscribe, send any mail to > "freebsd-arch-unsubscribe@freebsd.org"