Re: [RFC PATCH] ovl: keep some file attrubutions after copy-up

[Chengguang Xu <cgxu519@mykernel.net>]

 ---- 在 星期一, 2021-01-04 13:04:56 Amir Goldstein <amir73il@gmail.com> 撰写 ----
 > On Sat, Dec 26, 2020 at 12:48 PM Chengguang Xu <cgxu519@mykernel.net> 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.
 > 

You are right, I overlooked the case of dropping cache.

 > 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@miu.piliscsaba.redhat.com/
 > 
 > 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.

IIUC, the flags will still keep in overlayfs' inode after copy up until 
the inode cleaned by dropping cache. So I think your suggestion will be
helpful for the case of meta-copyup & dropping cache.

Hi Miklos

Is it worth to change like above?


 > 
 > ovl_ioctl_set_flags() triggers data copy up, so that will break the link
 > to lower flags anyway.

I think though ovl_ioctl_set_flags() triggers data copy up but the flags
will be set correctly to upper file, because chattr(1) will get the flags
first and set the whole flags(include original flags) to upper file.

Thanks,
Chengguang