On Thu, Apr 27, 2023 at 2:12 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> 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.
On second thought, I will add best effort lazy lowerdata lookup
in addition to the warning.
d_real_inode() does not expect errors or NULL and trace_uprobe
does not check for them either.
Thanks,
Amir.
