On Sun, May 14, 2023 at 10:13 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Thu, Apr 27, 2023 at 4:05 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > Introduce the format lowerdir=lower1:lower2::lowerdata1:lowerdata2
> > where the lower layers on the right of the :: separator are not merged
> > into the overlayfs merge dirs.
> >
> > The files in those layers are only meant to be accessible via absolute
> > redirect from metacopy files in lower layers. Following changes will
> > implement lookup in the data layers.
> >
> > This feature was requested for composefs ostree use case, where the
> > lower data layer should only be accessiable via absolute redirects
> > from metacopy inodes.
> >
> > The lower data layers are not required to a have a unique uuid or any
> > uuid at all, because they are never used to compose the overlayfs inode
> > st_ino/st_dev.
> >
> > Reviewed-by: Alexander Larsson <alexl@xxxxxxxxxx>
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > Documentation/filesystems/overlayfs.rst | 36 +++++++++++++++++++
> > fs/overlayfs/namei.c | 2 +-
> > fs/overlayfs/ovl_entry.h | 9 +++++
> > fs/overlayfs/super.c | 46 +++++++++++++++++++++----
> > 4 files changed, 85 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> > index 4c76fda07645..bc95343bafba 100644
> > --- a/Documentation/filesystems/overlayfs.rst
> > +++ b/Documentation/filesystems/overlayfs.rst
> > @@ -371,6 +371,42 @@ conflict with metacopy=on, and will result in an error.
> > [*] redirect_dir=follow only conflicts with metacopy=on if upperdir=... is
> > given.
> >
> > +
> > +Data-only lower layers
> > +----------------------
> > +
> > +With "metacopy" feature enabled, an overlayfs regular file may be a composition
> > +of information from up to three different layers:
> > +
> > + 1) metadata from a file in the upper layer
> > +
> > + 2) st_ino and st_dev object identifier from a file in a lower layer
> > +
> > + 3) data from a file in another lower layer (further below)
> > +
> > +The "lower data" file can be on any lower layer, except from the top most
> > +lower layer.
> > +
> > +Below the top most lower layer, any number of lower most layers may be defined
> > +as "data-only" lower layers, using the double colon ("::") separator.
> > +The double colon ("::") separator can only occur once and it must have a
> > +non-empty list of lower directory paths on the left and a non-empty
> > +list of "data-only" lower directory paths on the right.
> > +
> > +
> > +For example:
> > +
> > + mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1:/do2 /merged
> > +
> > +The paths of files in the "data-only" lower layers are not visible in the
> > +merged overlayfs directories and the metadata and st_ino/st_dev of files
> > +in the "data-only" lower layers are not visible in overlayfs inodes.
> > +
> > +Only the data of the files in the "data-only" lower layers may be visible
> > +when a "metacopy" file in one of the lower layers above it, has a "redirect"
> > +to the absolute path of the "lower data" file in the "data-only" lower layer.
> > +
> > +
> > Sharing and copying layers
> > --------------------------
> >
> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> > index e2b3c8f6753a..6bb07e1c01ee 100644
> > --- a/fs/overlayfs/namei.c
> > +++ b/fs/overlayfs/namei.c
> > @@ -356,7 +356,7 @@ int ovl_check_origin_fh(struct ovl_fs *ofs, struct ovl_fh *fh, bool connected,
> > struct dentry *origin = NULL;
> > int i;
> >
> > - for (i = 1; i < ofs->numlayer; i++) {
> > + for (i = 1; i <= ovl_numlowerlayer(ofs); i++) {
> > /*
> > * If lower fs uuid is not unique among lower fs we cannot match
> > * fh->uuid to layer.
> > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > index 548c93e030fc..93ff299da0dd 100644
> > --- a/fs/overlayfs/ovl_entry.h
> > +++ b/fs/overlayfs/ovl_entry.h
> > @@ -57,6 +57,8 @@ struct ovl_fs {
> > unsigned int numlayer;
> > /* Number of unique fs among layers including upper fs */
> > unsigned int numfs;
> > + /* Number of data-only lower layers */
> > + unsigned int numdatalayer;
> > const struct ovl_layer *layers;
> > struct ovl_sb *fs;
> > /* workbasedir is the path at workdir= mount option */
> > @@ -90,6 +92,13 @@ struct ovl_fs {
> > errseq_t errseq;
> > };
> >
> > +
> > +/* Number of lower layers, not including data-only layers */
> > +static inline unsigned int ovl_numlowerlayer(struct ovl_fs *ofs)
> > +{
> > + return ofs->numlayer - ofs->numdatalayer - 1;
> > +}
> > +
> > static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
> > {
> > return ofs->layers[0].mnt;
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 9b326b857ad6..988edb9e9d23 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -1576,6 +1576,16 @@ static int ovl_get_fsid(struct ovl_fs *ofs, const struct path *path)
> > return ofs->numfs++;
> > }
> >
> > +/*
> > + * The fsid after the last lower fsid is used for the data layers.
> > + * It is a "null fs" with a null sb, null uuid, and no pseudo dev.
> > + */
> > +static int ovl_get_data_fsid(struct ovl_fs *ofs)
> > +{
> > + return ofs->numfs;
> > +}
> > +
> > +
> > static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> > struct path *stack, unsigned int numlower,
> > struct ovl_layer *layers)
> > @@ -1583,11 +1593,14 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> > int err;
> > unsigned int i;
> >
> > - ofs->fs = kcalloc(numlower + 1, sizeof(struct ovl_sb), GFP_KERNEL);
> > + ofs->fs = kcalloc(numlower + 2, sizeof(struct ovl_sb), GFP_KERNEL);
> > if (ofs->fs == NULL)
> > return -ENOMEM;
> >
> > - /* idx/fsid 0 are reserved for upper fs even with lower only overlay */
> > + /*
> > + * idx/fsid 0 are reserved for upper fs even with lower only overlay
> > + * and the last fsid is reserved for "null fs" of the data layers.
> > + */
> > ofs->numfs++;
> >
> > /*
> > @@ -1612,7 +1625,10 @@ static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
> > struct inode *trap;
> > int fsid;
> >
> > - fsid = ovl_get_fsid(ofs, &stack[i]);
> > + if (i < numlower - ofs->numdatalayer)
> > + fsid = ovl_get_fsid(ofs, &stack[i]);
> > + else
> > + fsid = ovl_get_data_fsid(ofs);
> > if (fsid < 0)
> > return fsid;
> >
> > @@ -1700,6 +1716,7 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
> > int err;
> > struct path *stack = NULL;
> > struct ovl_path *lowerstack;
> > + unsigned int numlowerdata = 0;
> > unsigned int i;
> > struct ovl_entry *oe;
> >
> > @@ -1712,13 +1729,27 @@ static struct ovl_entry *ovl_get_lowerstack(struct super_block *sb,
> > if (!stack)
> > return ERR_PTR(-ENOMEM);
> >
> > - err = -EINVAL;
> > - for (i = 0; i < numlower; i++) {
> > + for (i = 0; i < numlower;) {
> > err = ovl_lower_dir(lower, &stack[i], ofs, &sb->s_stack_depth);
> > if (err)
> > goto out_err;
> >
> > lower = strchr(lower, '\0') + 1;
> > +
> > + i++;
> > + err = -EINVAL;
> > + /* :: separator indicates the start of lower data layers */
> > + if (!*lower && i < numlower && !numlowerdata) {
>
> FYI, kernel test bot reported a KASAN out-of-bounds access on !*lower
> because it is tested also when i == numlower and in any case, the test
> should be i < numlower - 1, so :: cannot be at the end of the list.
>
> Pushed a fix to branch ovl-lazy-lowerdata with an improved comment:
>
> /*
> * Empty lower layer path could mean :: separator that indicates
> * the start of lower data layers.
> * Only one :: separator is allowed and it has to have at least
> * one lowerdir to the left and one lowerdir to the right.
> */
> if (!numlowerdata && i < numlower - 1 && !*lower) {
>
>
FYI, rebased branch ovl-lazy-lowerdata onto v6.4-rc2.
Miklos, please let me know if you want me to re-post v3
with the KASAN warning fix above.
Thanks,
Amir.
