On Wed, Apr 26, 2023 at 5:57 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Wed, 12 Apr 2023 at 15:54, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > Make the code handle the case of numlower > 1 and missing lowerdata
> > dentry gracefully.
> >
> > Missing lowerdata dentry is an indication for lazy lookup of lowerdata
> > and in that case the lowerdata_redirect path is stored in ovl_inode.
> >
> > Following commits will defer lookup and perform the lazy lookup on
> > acccess.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> > fs/overlayfs/export.c | 2 +-
> > fs/overlayfs/file.c | 7 +++++++
> > fs/overlayfs/inode.c | 18 ++++++++++++++----
> > fs/overlayfs/super.c | 3 +++
> > fs/overlayfs/util.c | 2 +-
> > 5 files changed, 26 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> > index 9951c504fb8d..2498fa8311e3 100644
> > --- a/fs/overlayfs/export.c
> > +++ b/fs/overlayfs/export.c
> > @@ -343,7 +343,7 @@ static struct dentry *ovl_dentry_real_at(struct dentry *dentry, int idx)
> > if (!idx)
> > return ovl_dentry_upper(dentry);
> >
> > - for (i = 0; i < ovl_numlower(oe); i++) {
> > + for (i = 0; i < ovl_numlower(oe) && lowerstack[i].layer; i++) {
>
> Metacopy and NFS export are mutually exclusive, so this doesn't make sense.
>
OK.
>
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 3484f39a8f27..ef78abc21998 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
>
> ovl_d_real() calls ovl_dentry_lowerdata(). If triggered from
> file_dentry() it should be okay, since that is done on an open file
> (lazy lookup already perfromed). But it can also be called from
> d_real_inode(), the only caller of which is trace_uprobe. Is this
> going to be okay?
>
Not sure.
It's hard to imagine that trace_uprobe_create() is being called to place
a probe on a file at offset X without reading the content of symbols first.
I wonder if lazy lookup from within ovl_d_real(d, NULL) is acceptable?
It does look like the context of trace_uprobe() callers should be fine for lazy
lowerdata lookup.
> In any case a comment is needed at least.
>
I will leave it as is and add a comment.
Thanks,
Amir.
