Subject: Re: [PATCH] ext4: fix NULL pointer dereference while journal is
 aborted
To:     Jiufei Xue <jiufei.xue@linux.alibaba.com>,
        Joseph Qi <joseph.qi@linux.alibaba.com>,
        linux-ext4@vger.kernel.org, tytso@mit.edu, Jan Kara <jack@suse.cz>
References: <20190311063528.112996-1-jiufei.xue@linux.alibaba.com>
 <82869475-fc3b-441e-e87b-866e9aa1d93b@linux.alibaba.com>
 <600cf315-b3bc-75f4-5816-757eef4d6689@linux.alibaba.com>
From:   Jiufei Xue <xuejiufei@gmail.com>
Message-ID: <90b21c16-b7e2-31e7-ba1e-510cb3e45495@gmail.com>
Date:   Fri, 15 Mar 2019 09:18:32 +0800
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0)
 Gecko/20100101 Thunderbird/60.3.3
MIME-Version: 1.0
In-Reply-To: <600cf315-b3bc-75f4-5816-757eef4d6689@linux.alibaba.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Sender: linux-ext4-owner@vger.kernel.org
Precedence: bulk

Hi Jan,

Could you spend some time to review this patch and give some feedback?

Thanks a lot.

Jiufei

On 2019/3/12 下午7:00, Jiufei Xue wrote:
> Hi Ted,
>
> Would you please review this patch?
>
> Thanks,
> Jiufei
>
> On 2019/3/11 下午2:45, Joseph Qi wrote:
>>
>> On 19/3/11 14:35, Jiufei Xue wrote:
>>> We see the following NULL pointer dereference while running xfstests
>>> generic/475:
>>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
>>> PGD 8000000c84bad067 P4D 8000000c84bad067 PUD c84e62067 PMD 0
>>> Oops: 0000 [#1] SMP PTI
>>> CPU: 7 PID: 9886 Comm: fsstress Kdump: loaded Not tainted 5.0.0-rc8 #10
>>> RIP: 0010:ext4_do_update_inode+0x4ec/0x760
>>> ...
>>> Call Trace:
>>> ? jbd2_journal_get_write_access+0x42/0x50
>>> ? __ext4_journal_get_write_access+0x2c/0x70
>>> ? ext4_truncate+0x186/0x3f0
>>> ext4_mark_iloc_dirty+0x61/0x80
>>> ext4_mark_inode_dirty+0x62/0x1b0
>>> ext4_truncate+0x186/0x3f0
>>> ? unmap_mapping_pages+0x56/0x100
>>> ext4_setattr+0x817/0x8b0
>>> notify_change+0x1df/0x430
>>> do_truncate+0x5e/0x90
>>> ? generic_permission+0x12b/0x1a0
>>>
>>> This is triggered because the NULL pointer handle->h_transaction was
>>> dereferenced in function ext4_update_inode_fsync_trans().
>>> I found that the h_transaction was set to NULL in jbd2__journal_restart
>>> but failed to attached to a new transaction while the journal is aborted.
>>>
>>> Fix this by checking the handle before updating the inode.
>>>
>>> Signed-off-by: Jiufei Xue <jiufei.xue@linux.alibaba.com>
>> Looks good.
>> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
>>
>>> ---
>>>  fs/ext4/ext4_jbd2.h | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ext4/ext4_jbd2.h b/fs/ext4/ext4_jbd2.h
>>> index a1ac7e9245ec..75a5309f2231 100644
>>> --- a/fs/ext4/ext4_jbd2.h
>>> +++ b/fs/ext4/ext4_jbd2.h
>>> @@ -384,7 +384,7 @@ static inline void ext4_update_inode_fsync_trans(handle_t *handle,
>>>  {
>>>  	struct ext4_inode_info *ei = EXT4_I(inode);
>>>  
>>> -	if (ext4_handle_valid(handle)) {
>>> +	if (ext4_handle_valid(handle) && !is_handle_aborted(handle)) {
>>>  		ei->i_sync_tid = handle->h_transaction->t_tid;
>>>  		if (datasync)
>>>  			ei->i_datasync_tid = handle->h_transaction->t_tid;
>>>