Re: [PATCH] ubifs: Fix regression in ubifs_readdir()

[Jörg Krause]

Richard,

On Fri, 2016-10-28 at 19:07 +0200, Richard Weinberger wrote:
> JÃrg,
> 
> On 28.10.2016 18:19, JÃrg Krause wrote:
> > Hi,
> > 
> > On Fri, 2016-10-28 at 11:53 +0200, Richard Weinberger wrote:
> > > Commit c83ed4c9dbb35 ("ubifs: Abort readdir upon error") broke
> > > overlayfs support because the fix exposed an internal error
> > > code to VFS.
> > > 
> > > Reported-by: Peter Rosin <peda@xxxxxxxxxx>
> > > Tested-by: Peter Rosin <peda@xxxxxxxxxx>
> > > Reported-by: Ralph Sennhauser <ralph.sennhauser@xxxxxxxxx>
> > > Fixes: c83ed4c9dbb35 ("ubifs: Abort readdir upon error")
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > Signed-off-by: Richard Weinberger <richard@xxxxxx>
> > > ---
> > > Âfs/ubifs/dir.c | 8 ++++++++
> > > Â1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> > > index bd4a5e8ce441..ca16c5d7bab1 100644
> > > --- a/fs/ubifs/dir.c
> > > +++ b/fs/ubifs/dir.c
> > > @@ -543,6 +543,14 @@ static int ubifs_readdir(struct file *file,
> > > struct dir_context *ctx)
> > > Â
> > > Â	if (err != -ENOENT)
> > > Â		ubifs_err(c, "cannot find next direntry, error
> > > %d",
> > > err);
> > > +	else
> > > +		/*
> > > +		Â* -ENOENT is a non-fatal error in this context,
> > > the
> > > TNC uses
> > > +		Â* it to indicate that the cursor moved past the
> > > current directory
> > > +		Â* and readdir() has to stop.
> > > +		Â*/
> > > +		err = 0;
> > > +
> > > Â
> > > Â	/* 2 is a special value indicating that there are no
> > > more
> > > direntries */
> > > Â	ctx->pos = 2;
> > 
> > I'm not sure if it's related to the issue reported by Peter Rosin
> > and
> > Ralph Sennhauser, but I am still getting a kernel panic using UBIFS
> > with OverlayFS on Linux v4.9.0-rc2 with this patch applied:
> 
> Does reverting c83ed4c9dbb35 help?
> And are you 100% sure you applied the fix?

I double double checked. The fix was applied on the git tree, but the
compiler cache (I am using Buildroot with this option enabled) fooled
me by using an old copy. After disabling the compiler cache I got a
fixed build of the kernel. The panic is gone! Thanks!

> 
> Does the following WARN_ON() trigger?
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index ca16c5d7bab1..12ffc91f7ef8 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -554,6 +554,9 @@ static int ubifs_readdir(struct file *file,
> struct dir_context *ctx)
> 
> Â	/* 2 is a special value indicating that there are no more
> direntries */
> Â	ctx->pos = 2;
> +
> +	WARN_ON(err);
> +
> Â	return err;
> Â}

Best regards,
JÃrg Krause