Hi,
Over time there has been many requests for ext2 file flags such as
immutable, append-only, sync, noatime be implemented in XFS.
So, here it is. I have implemented all of the above flags in 2.4.20
and 2.4.21-rc7 XFS. The patch is relatively small and non-invasive,
and from my testing appears to cause no backwards compatibility issues
(older kernels ignore, and leave alone the new flags). Also
xfs_repair and xfs_check do not seem to have any problems with the new
flags either.
However during my testing I did discover 3 issues:
1: The Linux VFS contains a bug which allows timestamp modification of
immutable/append-only files. I have fixed this issue, and will send
the patch to the linux-kernel mailing list shortly. I have included
that patch in this mail for completeness.
2: xfsrestore breaks when immutable or append-only files are included
in the dump. The problem is xfsrestore restores the file inode flags
too soon, and then loses the access it needs to restore extended
attributes, uid, gid, and permissions. I am not familiar enough with
xfsrestore to really propose the fix, however i believe just changing
it to restore at the very least immutable/append-only flags very last
after all other restoration is complete should suffice to solve this
problem.
3: I just found an undocumented behavior with regards to ext2's file
flags; when set on a directory any new file/dir created will inherit
the flags of the parent directory. I am not entirely convinced this
is a good idea, at least for *all* flags. In ext2 this behavior has
been half broken for as near as i can tell, forever up to 2.4.20
(2.4.21 fixes it), the brokenness is that the vfs is not immediately
informed of any of the flags except sync, thus they don't actually go
into affect on new files until the inode is revalidated (which could
take quite some time). In addition ext2 only refrains from doing this
on symlinks, not device special files; this means if you create a
device node in a append-only directory the only way you will ever be
able to remove it is with debugfs. (ioctl() calls on a device file go
to the driver handling the node, not to the filesystem holding the
node, thus chattr won't help you).
My question on #3 is whether we want this behavior in XFS? my current
patch does NOT implement it. My opinion is that the only flag this
really makes sense for is sync, since setting the sync flag on a
directory does not actually do anything for you (it only affects
files), so making all new files created under a sync directory is
seemingly sensible (and probably what the user would expect, more or
less). this might make sense for noatime too, but I am less convinced
of this. I am definitely not convinced its a sensible idea for
append-only since it ends up allowing non-privileged users to create
append-only files, which is not normally permitted. Also
automatically setting the append-only flag on a newly created file
won't necessarily make it append-only immediately, since a caller doing
open("append-only-dir/file" O_RDWR|O_CREAT) will retain full
read/write access so long as the file descriptor is kept open (the
same is true if you chmod() a file, open descriptors retain access
even if they could not get it on a new open()).
So for now I submit the patch without the ext2 inheritance behavior,
if you believe we should implement the ext2 behavior either partially,
or fully I should be able to do that (though a suggestion on the best
place in XFS to do this task would be welcome).
I have also implemented the EXT2_IOC_SETFLAGS/EXT2_IOC_GETFLAGS
ioctl()s so chattr/lsattr will work on xfs, this part of the patch is
optional, if excluded then either lsattr/chattr would need to be
modified to use XFS_IOC_FSSETXATTR and XFS_IOC_FSGETXATTR ioctls.
I have included a (somewhat hideous) test program to check as many
things as i could think of to ensure immutable/append-only are
properly enforced, the only thing lacking in it is tests of all the
XFS ioctls, perhaps someone familiar with xfs ioctls could add these
(i tried to do at least the libhandle cases, but couldn't get
libhandle to work).
Important: this test program creates a test area with full world
read/write permissions to ensure regular file permissions are not
tainting the test, therefore it is NOT SAFE to run on systems with
untrusted users. the usage is t_immutable -c /some/dir which will
create a test area as /some/dir and run tests in it. t_immutable -C
will create the test area without running any tests, and t_immutable
-r will remove a test area. t_immutable /some/dir will run the tests
on a previously created test area.
Note if you run t_immutable on an ext2 filesystem in 2.4.21 kernels
you will need to use debugfs to get rid of the device node it creates
(same on kernels prior to 2.4.21 if you leave the test area around
long enough). see above for why.
feedback is welcome.
diffstat output follows:
fs/xfs/linux/xfs_ext2_ioctl.h | 45 +++++++++++++++++++
fs/xfs/linux/xfs_ioctl.c | 99 ++++++++++++++++++++++++++++++++++++++++++
fs/xfs/linux/xfs_iops.c | 6 ++
fs/xfs/linux/xfs_super.c | 16 ++++++
fs/xfs/linux/xfs_vnode.c | 17 +++++++
fs/xfs/xfs_acl.c | 2
fs/xfs/xfs_cap.c | 2
fs/xfs/xfs_dinode.h | 24 +++++++---
fs/xfs/xfs_fs.h | 7 ++
fs/xfs/xfs_inode.c | 5 +-
fs/xfs/xfs_itable.c | 8 +++
fs/xfs/xfs_vnodeops.c | 16 ++++++
12 files changed, 238 insertions(+), 9 deletions(-)
--
Ethan Benson
http://www.alaska.net/~erbenson/
xfs-xflags.diff
Description: Text document
open.c.diff
Description: Text document
|