On 2024/5/16 16:55, sunshijie wrote:
> When thread A calls kill_f2fs_super, Thread A first executes the code sbi->node_inode = NULL;
> Then thread A may submit a bio to the function iput(sbi->meta_inode);
> Then thread A enters the process D state,
> Now that the bio submitted by thread A is complete, it calls f2fs_write_end_io and may trigger null-ptr-deref in NODE_MAPPING.
I didn't get it, if there is no cp_err, f2fs_write_checkpoint() in
f2fs_put_super() will flush all dirty pages of node_inode, if there is
cp_err, below flow will keep all dirty pages being truncated, and
there is sanity check on all types of dirty pages.
/* our cp_error case, we can wait for any writeback page */
f2fs_flush_merged_writes(sbi);
f2fs_wait_on_all_pages(sbi, F2FS_WB_CP_DATA);
if (err || f2fs_cp_error(sbi)) {
truncate_inode_pages_final(NODE_MAPPING(sbi));
truncate_inode_pages_final(META_MAPPING(sbi));
}
for (i = 0; i < NR_COUNT_TYPE; i++) {
if (!get_pages(sbi, i))
continue;
f2fs_err(sbi, "detect filesystem reference count leak during "
"umount, type: %d, count: %lld", i, get_pages(sbi, i));
f2fs_bug_on(sbi, 1);
}
So, is there any missing case that dirty page of node_inode is missed by
f2fs_put_super()?
Thanks,
>
> Thread A IRQ context
> - f2fs_put_super
> - sbi->node_inode = NULL;
> - iput(sbi->meta_inode);
> - iput_final
> - write_inode_now
> - writeback_single_inode
> - __writeback_single_inode
> - filemap_fdatawait
> - filemap_fdatawait_range
> - __kcfi_typeid_free_transhuge_page
> - __filemap_fdatawait_range
> - wait_on_page_writeback
> - folio_wait_writeback
> - folio_wait_bit
> - folio_wait_bit_common
> - io_schedule
>
> - __handle_irq_event_percpu
> - ufs_qcom_mcq_esi_handler
> - ufshcd_mcq_poll_cqe_nolock
> - ufshcd_compl_one_cqe
> - scsi_done
> - scsi_done_internal
> - blk_mq_complete_request
> - scsi_complete
> - scsi_finish_command
> - scsi_io_completion
> - scsi_end_request
> - blk_update_request
> - bio_endio
> - f2fs_write_end_io
> - NODE_MAPPING(sbi)
>
> Signed-off-by: sunshijie <[email protected]>
> ---
> fs/f2fs/super.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index adffc9b80a9c..aeb085e11f9a 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -1641,12 +1641,12 @@ static void f2fs_put_super(struct super_block *sb)
>
> f2fs_destroy_compress_inode(sbi);
>
> - iput(sbi->node_inode);
> - sbi->node_inode = NULL;
> -
> iput(sbi->meta_inode);
> sbi->meta_inode = NULL;
>
> + iput(sbi->node_inode);
> + sbi->node_inode = NULL;
> +
> mutex_unlock(&sbi->umount_mutex);
>
> /*