Linux-f2fs-devel Archive mirror
 help / color / mirror / Atom feed
From: Chao Yu <chao@kernel.org>
To: sunshijie <sunshijie@xiaomi.corp-partner.google.com>,
	jaegeuk@kernel.org,  linux-f2fs-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org
Cc: sunshijie <sunshijie@xiaomi.com>
Subject: Re: [f2fs-dev] [PATCH] f2fs: fix panic in f2fs_put_super
Date: Thu, 16 May 2024 17:32:16 +0800	[thread overview]
Message-ID: <30a5f29b-886f-4ee7-93d1-8c3e1bb8bb49@kernel.org> (raw)
In-Reply-To: <20240516085512.1082640-1-sunshijie@xiaomi.com>

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 <sunshijie@xiaomi.com>
> ---
>   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);
>   
>   	/*


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2024-05-16  9:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-16  8:55 [f2fs-dev] [PATCH] f2fs: fix panic in f2fs_put_super sunshijie via Linux-f2fs-devel
2024-05-16  9:32 ` Chao Yu [this message]
  -- strict thread matches above, loose matches on Subject: below --
2024-05-16 10:13 sunshijie via Linux-f2fs-devel
2024-05-16  8:40 sunshijie via Linux-f2fs-devel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=30a5f29b-886f-4ee7-93d1-8c3e1bb8bb49@kernel.org \
    --to=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sunshijie@xiaomi.com \
    --cc=sunshijie@xiaomi.corp-partner.google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).