On Sun, Oct 18, 2015 at 11:44 PM, Theodore Ts'o <tytso@xxxxxxx> wrote: > On Sun, Oct 18, 2015 at 10:46:23PM +0200, Andreas Gruenbacher wrote: >> > The only question is whether we pay attention to the richacl acl's at >> > all. One thing that's not clear to me is what VFS is supposed to do >> > if an inode has both an Posix ACL xattr and a Richacl xattr at the >> > same time. >> >> The VFS will either look for POSIX ACLs or for a richacl; it won't >> even notice if both are present. > > How does this work in practice? Does it look for richacl's first, and > if it doesn't find it, it will then look for a Posix ACL, or vice > versa? The filesystem sets the MS_POSIXACL super-block flag for POSIX ACLs or the MS_RICHACL super-block flag for richacls. These flags are checked with the IS_POSIXACL(inode) and IS_RICHACL(inode) macros. >> Right now, filesystems that e2fsck is perfectly happy with can still >> cause errors when used. It would be nice to fix that. >> >> With POSIX ACLs, this problem is slightly less severe because the ACL >> isn't looked at for the owner; it would even be possible to replace a >> corrupted POSIX ACL. Richacls unfortunately don't allow this >> optimization. > > Is there code we can use to verify a richacl, and if it's corrupted, > what are the options about how we can fix it? Or do we just remove > it, and just use the inode's i_uid field for the owner instead of > whatever might be in the richacl? The on-disk format is relatively simple, it's the same on ext4 and on xfs and hopefully will be the same on other filesystems as well. That code could be shared. If a richacl is found to be corrupt, a safe way of recovering would be to remove the richacl and clear the S_IRWXUGO mode bits. I don't think trying to fix ACLs would make sense. The i_uid field is unrelated. > Ideally, if you can send the patch to add support to validate / fix > Richacl's in e2fsck, that would be great. I can send the validation code; not sure how removing corrupt xattrs would fit into e2fsck though. Can e2fsck remove individual xattrs? >> This really should be a feature flag and not a mount option, it just >> doesn't make sense to switch at mount time. >> >> From this discussion, I'm even more convinced that we should use an >> incompat feature rather than a ro-incompat feature. > > OK, let's go with that. Okay, good. Thanks, Andreas -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
