Date: Fri, 8 Mar 2013 19:11:44 -0500 (EST) From: Rick Macklem <rmacklem@uoguelph.ca> To: "Kenneth D. Merry" <ken@FreeBSD.org> Cc: arch@FreeBSD.org, fs@FreeBSD.org Subject: Re: patches to add new stat(2) file flags Message-ID: <37540270.3721223.1362787904494.JavaMail.root@erie.cs.uoguelph.ca> In-Reply-To: <20130308232155.GA47062@nargothrond.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?37540270.3721223.1362787904494.JavaMail.root>