Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 23 Jun 2021 19:38:42 +0300
From:   Pavel Skripkin <paskripkin@gmail.com>
To:     Dave Kleikamp <dave.kleikamp@oracle.com>
Cc:     shaggy@kernel.org, jfs-discussion@lists.sourceforge.net,
        linux-kernel@vger.kernel.org,
        syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
Subject: Re: [PATCH] jfs: fix GPF in diFree
Message-ID: <20210623193842.5f164966@gmail.com>
In-Reply-To: <319afbd1-afc0-bae3-c446-3530505e7b21@oracle.com>
References: <20210606142405.31047-1-paskripkin@gmail.com>
        <319afbd1-afc0-bae3-c446-3530505e7b21@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 23 Jun 2021 09:13:07 -0500
Dave Kleikamp <dave.kleikamp@oracle.com> wrote:

> On 6/6/21 9:24 AM, Pavel Skripkin wrote:
> > Avoid passing inode with
> > JFS_SBI(inode->i_sb)->ipimap == NULL to
> > diFree()[1]. GFP will appear:
> 
> I'm a little curious how we get as far as creating and freeing 
> non-special inodes if ipimap == NULL.
> 
> > 
> > 	struct inode *ipimap = JFS_SBI(ip->i_sb)->ipimap;
> > 	struct inomap *imap = JFS_IP(ipimap)->i_imap;
> > 
> > JFS_IP() will return invalid pointer when ipimap == NULL
> > 
> > Call Trace:
> >   diFree+0x13d/0x2dc0 fs/jfs/jfs_imap.c:853 [1]
> >   jfs_evict_inode+0x2c9/0x370 fs/jfs/inode.c:154
> >   evict+0x2ed/0x750 fs/inode.c:578
> >   iput_final fs/inode.c:1654 [inline]
> >   iput.part.0+0x3fe/0x820 fs/inode.c:1680
> >   iput+0x58/0x70 fs/inode.c:1670
> 
> Is there more to the stack trace? Is this part of a failed mount()?
> 

Hi, Dave!

Yes, it was caused by mount fail. Log:

[  924.076873][ T8430] jfs_mount: diMount(ipaimap) failed w/rc = -5

So, it's errout21 label in jfs_mount(). I guess, It's early failure and
some fields wasn't initialized properly. I don't really remember my
debug results, because it was a long time ago, but I can do some debug
work again if needed!


Thanks for feedback!

> > 
> > Reported-and-tested-by:
> > syzbot+0a89a7b56db04c21a656@syzkaller.appspotmail.com
> > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>ipimap == NULL
> 
> I don't doubt that this happened, so I'll apply the patch which is 
> obviously safe.
> 
> > ---
> >   fs/jfs/inode.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/jfs/inode.c b/fs/jfs/inode.c
> > index 6f65bfa9f18d..b0eb9c85eea0 100644
> > --- a/fs/jfs/inode.c
> > +++ b/fs/jfs/inode.c
> > @@ -151,7 +151,8 @@ void jfs_evict_inode(struct inode *inode)
> >   			if (test_cflag(COMMIT_Freewmap, inode))
> >   				jfs_free_zero_link(inode);
> >   
> > -			diFree(inode);
> > +			if (JFS_SBI(inode->i_sb)->ipimap)
> > +				diFree(inode);
> >   
> >   			/*
> >   			 * Free the inode from the quota
> > allocation.
> > 


With regards,
Pavel Skripkin