Date: Fri, 19 Apr 2013 22:53:50 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> 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: <20130419215624.L1262@besplex.bde.org> In-Reply-To: <20130418184951.GA18777@nargothrond.kdm.org> References: <20130307000533.GA38950@nargothrond.kdm.org> <20130307222553.P981@besplex.bde.org> <20130308232155.GA47062@nargothrond.kdm.org> <20130310181127.D2309@besplex.bde.org> <20130409190838.GA60733@nargothrond.kdm.org> <20130418184951.GA18777@nargothrond.kdm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 18 Apr 2013, Kenneth D. Merry wrote:
> On Tue, Apr 09, 2013 at 13:08:38 -0600, Kenneth D. Merry wrote:
>> ...
>> Okay, I think these issues should now be fixed. We now refuse to change
>> attributes only on the root directory. And I updatd deupdat() to do the
>> same.
>>
>> When a directory is created or a file is added, the archive bit is not
>> changed on the directory. Not sure if we need to do that or not. (Simply
>> changing msdosfs_mkdir() to set ATTR_ARCHIVE was not enough to get the
>> archive bit set on directory creation.)
>
> Bruce, any comment on this?
I didn't get around to looking at it closely. Just had a quick look at
the msdosfs parts.
Apparently we are already doing the same as WinXP for ATTR_ARCHIVE on
directories. Not the right thing, but:
- don't set it on directory creation
- don't set it on directory modification
- allow setting and clearing it (with your changes).
@ *** src/lib/libc/sys/chflags.2.orig
@ --- src/lib/libc/sys/chflags.2
@ ***************
@ *** 112,137 ****
@ ...
@ --- 112,170 ----
@ ...
@ + .It Dv UF_IMMUTABLE
@ + The file may not be changed.
@ + Filesystems may use this flag to maintain compatibility with the DOS, Windows
@ + and CIFS FILE_ATTRIBUTE_READONLY attribute.
msdosfs doesn't use this yet. It uses ATTR_READONLY, and doesn't map this
to or from UF_IMMUTABLE. I think I want ATTR_READONLY to be a flag and
not affect the file permissions (just like immutable flags normally don't
affect the file permissions.
Does CIFS FILE_ATTRIBUTE_READONLY have exactly the same semantics as
IMMUTABLE? That is, does it prevent all operations on the file and the
file's metadata except read()? For IMMUTABLE, the other operations that
it disallows include setattr(), rename() and unlink().
Well it doesn't in WinXP using Cygwin. I made a directory with attributes
+R, and this didn't prevent creating files in the directory or rmdir of
the directory. Even attributes +R +H +S didn't prevent these operations.
Maybe +R isn't really used for directories, like +A. Then for a file with
+R +H +S:
- rm asked before deleting it (+R changed its fake permissions from
rw-r--r-- to r--r--r--).
- touching it succeeded
- attrib on it succeeded
- writing it failed.
So it seems that in WinXP, ATTR_READONLY is ignored for directories, and
more like the !writeable permission than the immutable flag.
@ *** src/sys/fs/msdosfs/msdosfs_denode.c.orig
@ --- src/sys/fs/msdosfs/msdosfs_denode.c
@ ***************
@ *** 300,307 ****
@ if ((dep->de_flag & DE_MODIFIED) == 0)
@ return (0);
@ dep->de_flag &= ~DE_MODIFIED;
@ ! if (dep->de_Attributes & ATTR_DIRECTORY)
@ ! return (0);
@ if (dep->de_refcnt <= 0)
@ return (0);
@ error = readde(dep, &bp, &dirp);
@ --- 300,309 ----
@ if ((dep->de_flag & DE_MODIFIED) == 0)
@ return (0);
@ dep->de_flag &= ~DE_MODIFIED;
@ ! /* Was: silently ignore attribute changes for all dirs. */
@ ! if (DETOV(dep)->v_vflag & VV_ROOT)
@ ! return (EINVAL);
@ ! /* Otherwise valid. */
Clean up the comments a bit. Say nothing, or that all attributes apply
to all directories except the root directory.
Perhaps the VV_ROOT case is unreachable because callers filter out this
case. I have a debugger trap for it.
@ if (dep->de_refcnt <= 0)
@ return (0);
@ error = readde(dep, &bp, &dirp);
@ *** src/sys/fs/msdosfs/msdosfs_vnops.c.orig
@ --- src/sys/fs/msdosfs/msdosfs_vnops.c
@ ***************
@ *** 398,403 ****
@ --- 402,418 ----
@ if (vap->va_flags != VNOVAL) {
@ if (vp->v_mount->mnt_flag & MNT_RDONLY)
@ return (EROFS);
@ + /*
@ + * We don't allow setting attributes on the root directory,
@ + * because according to Bruce Evans: "The special case for
@ + * the root directory is because before FAT32, the root
@ + * directory didn't have an entry for itself (and was
@ + * otherwise special). With FAT32, the root directory is
@ + * not so special, but still doesn't have an entry for itself."
@ + */
@ + if (vp->v_vflag & VV_ROOT)
@ + return (EINVAL);
@ +
@ if (cred->cr_uid != pmp->pm_uid) {
@ error = priv_check_cred(cred, PRIV_VFS_ADMIN, 0);
@ if (error)
No need to give the source.
I prefer the do this check after the permissions check, but if it is done
early then it is best done as a single check for all attributes in
msdosfs_settattr() and not just for flags. Currently there is:
- no check for ownerships. We only allow null changes to ownerships. With
no check like the above, we allow them even for the root directory, while
the above disallows null changes to flags for the root directory.
- for truncate(), the error is EISDIR for all directories.
- for file times, we silently ignore changes for all directories, after doing
permissions checks. Only the root directory should be special.
- for file permissions, we handle directories as for file times. Now the
only possible non-null change is of ATTR_READONLY, and since this
apparently has no effect in WinXP, ignorig changing it for directories
is best.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130419215624.L1262>