On Sun, 6 Jun 2021 at 16:46, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > When a lower file has immutable/append-only attributes, the behavior of > overlayfs post copy up is inconsistent. > > Immediattely after copy up, ovl inode still has the S_IMMUTABLE/S_APPEND > inode flags copied from lower inode, so vfs code still treats the ovl > inode as immutable/append-only. After ovl inode evict or mount cycle, > the ovl inode does not have these inode flags anymore. > > We cannot copy up the immutable and append-only fileattr flags, because > immutable/append-only inodes cannot be linked and because overlayfs will > not be able to set overlay.* xattr on the upper inodes. Ugh. > Instead, if any of the fileattr flags of interest exist on the lower > inode, we set an xattr overlay.xflags on the upper inode as an indication > to merge the origin inode fileattr flags on lookup. > > This gives consistent behavior post copy up regardless of inode eviction > from cache. > > When user sets new fileattr flags, we break the connection with the > origin fileattr by removing the overlay.xflags xattr. > > Note that having the S_IMMUTABLE/S_APPEND on the ovl inode does not > provide the same level of protection as setting those flags on the real > upper inode, because some filesystem check those flags internally in > addition or instead of the vfs checks (e.g. btrfs_may_delete()), but > that is the way it has always been for overlayfs. That's fine, underlying filesystem is just a backing store. > As can be seen in the comment above ovl_check_origin_xflags(), the > "xflags merge" feature is designed to solve other non-standard behavior > issues related to immutable directories and hardlinks in the future, but > this commit does not bother to fix those cases because those are corner > cases that are probably not so important to fix. > > A word about the design decision to merge the origin and upper xflags - > Because we do not copy up fileattr and because fileattr_set breaks the > link to origin xflags, the only cases where origin and upper inodes both > have xflags is if upper inode was modified not via overlayfs or if the > system crashed during ovl_fileattr_set() before removing the > overlay.xflags xattr. In both cases, modifiying the upper inode is not > going to be permitted, so it is better to reflect this in the overlay > inode flags. So why not implement the non-merge (#3) behavior unconditionally? That would solve all issues related to fileattr, right? Thanks, Miklos
