On Thu, Sep 7, 2023 at 11:44 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
>
> We use the "overlay.whiteouts" to mark any directory in a lower dir that may
> contain "overlay.whiteout" files (xwhiteouts). This works even if other layers
> overlap that directory in a mount.
>
> For example, take these files in three layers:
>
> layer2/dir/ - overlay.whiteouts
> layer2/dir/file - overlay.whiteout
> layer1/dir/
> layer1/dir/file
>
> An overlayfs mount with -o lowerdir=layer2:layer1 would have a whiteout in
> layer2.
>
> However, suppose you wanted to put this inside an overlayfs layer (say
> "layerA"). I.e. you want to escape the whiteouts above so that when the new
> layer is mounted using overlayfs the mount shows the above content. The natural
> approach is to just take each layer and escape it:
>
> layerA/layer2/dir/ - overlay.overlay.whiteouts
> layerA/layer2/dir/file - overlay.overlay.whiteout
> layerA/layer1/dir/
> layerA/layer1/dir/file
>
> This initially seems to work, however if there is another lowerdir (say
> "layerB") that overlaps the xwhiteouts dir, then this runs into problem:
>
> layerB/layer2/dir/ - **NO overlay.overlay.whiteouts **
> layerA/layer2/dir/ - overlay.overlay.whiteouts
> layerA/layer2/dir/file - overlay.overlay.whiteout
> layerA/layer1/dir/
> layerA/layer1/dir/file
>
> If you mount this with -o lowerdir=layerB:layerA, then in the final mount,
> there will be no overlay.whiteouts xattrs on the "layer2/dir" merged
> directory, because the topmost lower dir xattrs win.
>
> We would like this to work as is, to avoid having layer escaping depend on other
> layers. So, to fix this up we special case the reading of escaped
> "overlay.whiteouts" xattrs by looking in all layers for the first hit.
>
I have a few issues with this special casing:
1. Miklos did not speak his opinion yet
2. I don't like special casing by suffix nor special casing a single xattr
3. I can think of example like this:
layerB/layer2/dir/file3
layerB/layer2/dir/ - **NO overlay.overlay.opaque **
layerA/layer2/dir/ - overlay.overlay.opaque
layerA/layer2/dir/file2
layerA/layer1/dir/
layerA/layer1/dir/file1
LayerB doesn't have layer1 so it does not know that
opaque is needed, therefore opaque needs to be merged
as well in this case.
Can we employ the logic of ovl_xattr_get_first() for any
escaped xattr prefix on a merge dir?
Thanks,
Amir.
> Signed-off-by: Alexander Larsson <alexl@xxxxxxxxxx>
> ---
> fs/overlayfs/xattrs.c | 46 ++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/xattrs.c b/fs/overlayfs/xattrs.c
> index 27b31f812eb1..9e5f50ba333d 100644
> --- a/fs/overlayfs/xattrs.c
> +++ b/fs/overlayfs/xattrs.c
> @@ -92,6 +92,25 @@ static int ovl_xattr_get(struct dentry *dentry, struct inode *inode, const char
> return res;
> }
>
> +static int ovl_xattr_get_first(struct dentry *dentry, struct inode *inode, const char *name,
> + void *value, size_t size)
> +{
> + const struct cred *old_cred;
> + struct path realpath;
> + int idx, next;
> + int res = -ENODATA;
> +
> + old_cred = ovl_override_creds(dentry->d_sb);
> + for (idx = 0; idx != -1; idx = next) {
> + next = ovl_path_next(idx, dentry, &realpath);
> + res = vfs_getxattr(mnt_idmap(realpath.mnt), realpath.dentry, name, value, size);
> + if (res != -ENODATA && res != -EOPNOTSUPP)
> + break;
> + }
> + revert_creds(old_cred);
> + return res;
> +}
> +
> static bool ovl_can_list(struct super_block *sb, const char *s)
> {
> /* Never list non-escaped private (.overlay) */
> @@ -176,6 +195,18 @@ static char *ovl_xattr_escape_name(const char *prefix, const char *name)
> return escaped;
> }
>
> +
> +static int str_ends_with(const char *s, const char *sub)
> +{
> + int slen = strlen(s);
> + int sublen = strlen(sub);
> +
> + if (sublen > slen)
> + return 0;
> +
> + return !memcmp(s + slen - sublen, sub, sublen);
> +}
> +
> static int ovl_own_xattr_get(const struct xattr_handler *handler,
> struct dentry *dentry, struct inode *inode,
> const char *name, void *buffer, size_t size)
> @@ -187,7 +218,20 @@ static int ovl_own_xattr_get(const struct xattr_handler *handler,
> if (IS_ERR(escaped))
> return PTR_ERR(escaped);
>
> - r = ovl_xattr_get(dentry, inode, escaped, buffer, size);
> + /*
> + * Escaped "overlay.whiteouts" directories need to be combined across layers.
> + * For example, if a lower layer contains an escaped "overlay.whiteout"
> + * its parent directory will be marked with an escaped "overlay.whiteouts".
> + * The merged directory will contain a (now non-escaped) whiteout, and its
> + * parent should therefore be marked too. However, if a layer above the marked
> + * one has covers the same directory but without whiteouts the covering directory
> + * would not be marged, and thus the merged directory would not be marked.
> + */
> + if (d_is_dir(dentry) &&
> + str_ends_with(escaped, "overlay.whiteouts"))
> + r = ovl_xattr_get_first(dentry, inode, escaped, buffer, size);
> + else
> + r = ovl_xattr_get(dentry, inode, escaped, buffer, size);
>
> kfree(escaped);
>
> --
> 2.41.0
>
