Re: [PATCH] jfs: fix GPF in diFree

[Dave Kleikamp <dave.kleikamp@oracle.com>]

On 6/23/21 11:38 AM, Pavel Skripkin wrote:
> 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. I don't want to ask you to spend any more time on this. Sorry I 
didn't get back sooner. Thanks for the fix!

Shaggy

> 
> 
> 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
>