Copying Miklos' another email. On Mon, Jul 06, 2020 at 03:15:06PM +0300, Amir Goldstein wrote: > On Sun, Jun 21, 2020 at 9:38 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > Mounting with nfs_export=on, xfstests overlay/031 triggers a kernel panic > > since v5.8-rc1 overlayfs updates. > > Ping. > > This is a kernel OOPS regression in v5.8-rc1. > > Push tested branch ovl-fixes based on v5.8-rc3 with this change. > It contains two other non-critical fixes, which are pretty obvious, > so you may want to consider taking them as well. > > Thanks, > Amir. > > > > > overlayfs: orphan index entry (index/00fb1..., ftype=4000, nlink=2) > > BUG: kernel NULL pointer dereference, address: 0000000000000030 > > RIP: 0010:ovl_cleanup_and_whiteout+0x28/0x220 [overlay] > > > > Bisect point at commit c21c839b8448 ("ovl: whiteout inode sharing") > > > > Minimal reproducer: > > -------------------------------------------------- > > rm -rf l u w m > > mkdir -p l u w m > > mkdir -p l/testdir > > touch l/testdir/testfile > > mount -t overlay -o lowerdir=l,upperdir=u,workdir=w,nfs_export=on overlay m > > echo 1 > m/testdir/testfile > > umount m > > rm -rf u/testdir > > mount -t overlay -o lowerdir=l,upperdir=u,workdir=w,nfs_export=on overlay m > > umount m > > -------------------------------------------------- > > > > When mount with nfs_export=on, and fail to verify an orphan index, we're > > cleaning this index from indexdir by calling ovl_cleanup_and_whiteout(). > > This dereferences ofs->workdir, that was earlier set to NULL. > > > > The design was that ovl->workdir will point at ovl->indexdir, but we are > > assigning ofs->indexdir to ofs->workdir only after ovl_indexdir_cleanup(). > > There is no reason not to do it sooner, because once we get success from > > ofs->indexdir = ovl_workdir_create(... there is no turning back. > > > > Reported-and-tested-by: Murphy Zhou <jencce.kernel@xxxxxxxxx> > > Fixes: commit c21c839b8448 ("ovl: whiteout inode sharing") > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > > --- > > fs/overlayfs/super.c | 16 +++++++--------- > > 1 file changed, 7 insertions(+), 9 deletions(-) > > > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c > > index 91476bc422f9..15939ab39c1c 100644 > > --- a/fs/overlayfs/super.c > > +++ b/fs/overlayfs/super.c > > @@ -1354,6 +1354,12 @@ static int ovl_get_indexdir(struct super_block *sb, struct ovl_fs *ofs, > > > > ofs->indexdir = ovl_workdir_create(ofs, OVL_INDEXDIR_NAME, true); > > if (ofs->indexdir) { > > + /* index dir will act also as workdir */ > > + iput(ofs->workdir_trap); > > + ofs->workdir_trap = NULL; > > + dput(ofs->workdir); > > + ofs->workdir = dget(ofs->indexdir); > > + > > err = ovl_setup_trap(sb, ofs->indexdir, &ofs->indexdir_trap, > > "indexdir"); > > if (err) > > @@ -1843,20 +1849,12 @@ static int ovl_fill_super(struct super_block *sb, void *data, int silent) > > sb->s_flags |= SB_RDONLY; > > > > if (!(ovl_force_readonly(ofs)) && ofs->config.index) { > > - /* index dir will act also as workdir */ > > - dput(ofs->workdir); > > - ofs->workdir = NULL; > > - iput(ofs->workdir_trap); > > - ofs->workdir_trap = NULL; > > - > > err = ovl_get_indexdir(sb, ofs, oe, &upperpath); > > if (err) > > goto out_free_oe; > > > > /* Force r/o mount with no index dir */ > > - if (ofs->indexdir) > > - ofs->workdir = dget(ofs->indexdir); > > - else > > + if (!ofs->indexdir) > > sb->s_flags |= SB_RDONLY; > > } > > > > -- > > 2.17.1 > > -- Murphy