On Sat, Dec 26, 2020 at 12:48 PM Chengguang Xu <cgxu519@xxxxxxxxxxxx> wrote: > > Currently after copy-up, upper file will lose most of file > attributions except copy-up triggered by setting fsflags. > Because ioctl operation of underlying file systems does not > expect calling from kernel component, it seems hard to > copy fsflags during copy-up. > > Overlayfs keeps limited attributions(append-only, etc) in it's > inode flags after successfully updating attributions. so ater > copy-up, lsattr(1) does not show correct result but overlayfs > can still prohibit ramdom write for those files which originally > have append-only attribution. However, recently I found this > protection can be easily broken in below operations. > > 1, Set append attribution to lower file. > 2, Mount overlayfs. > 3, Trigger copy-up by data append. > 4, Set noatime attributtion to the file. > 5, The file is random writable. > > This patch tries to keep some file attributions after copy-up > so that overlayfs keeps compatible behavior with local filesystem > as much as possible. > This approach seems quite wrong. For one thing, mount cycle overlay or drop caches will result in loss of append only flag after copy-up, so this is not a security fix. Second, Miklos has already proposed a much more profound change to address this and similar issues [1] and he has already made some changes to ioctl handler to master doesn't have ovl_iflags_to_fsflags(). [1] https://lore.kernel.org/linux-fsdevel/20201123141207.GC327006@xxxxxxxxxxxxxxxxxxxxxxxxx/ One more thing. It seems like ovl_copyflags() in ovl_inode_init() would have been better to copy from ovl_inode_realdata() inode instead of ovl_inode_real(). This way, copy up still loses the append-only flag, but metacopy up does not. So at least for the common use case of containers that chown -R won't cause losing all the file flags. ovl_ioctl_set_flags() triggers data copy up, so that will break the link to lower flags anyway. I also noticed that we can lose the immutable flag on lower directory when copying up parents and it's probably not the only way to lose an immutable flag on a lower file/dir. Thanks, Amir.
