From: Andrew Morton via Ocfs2-devel <ocfs2-devel@oss.oracle.com> To: mm-commits@vger.kernel.org, piaojun@huawei.com, mark@fasheh.com, junxiao.bi@oracle.com, joseph.qi@linux.alibaba.com, jlbec@evilplan.org, heming.zhao@suse.com, ghe@suse.com, gechangwei@live.cn, ocfs2-devel@oss.oracle.com, akpm@linux-foundation.org Subject: [Ocfs2-devel] [merged mm-nonmm-stable] ocfs2-fix-mounting-crash-if-journal-is-not-alloced.patch removed from -mm tree Date: Mon, 09 May 2022 21:17:27 -0700 [thread overview] Message-ID: <20220510041728.165E3C385C7@smtp.kernel.org> (raw) The quilt patch titled Subject: ocfs2: fix mounting crash if journal is not alloced has been removed from the -mm tree. Its filename was ocfs2-fix-mounting-crash-if-journal-is-not-alloced.patch This patch was dropped because it was merged into the mm-nonmm-stable branch of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm ------------------------------------------------------ From: Heming Zhao via Ocfs2-devel <ocfs2-devel@oss.oracle.com> Subject: ocfs2: fix mounting crash if journal is not alloced Patch series "rewrite error handling during mounting stage". This patch (of 5): After commit da5e7c87827e8 ("ocfs2: cleanup journal init and shutdown"), journal init later than before, it makes NULL pointer access in free routine. Crash flow: ocfs2_fill_super + ocfs2_mount_volume | + ocfs2_dlm_init //fail & return, osb->journal is NULL. | + ... | + ocfs2_check_volume //no chance to init osb->journal | + ... + ocfs2_dismount_volume ocfs2_release_system_inodes ... evict ... ocfs2_clear_inode ocfs2_checkpoint_inode ocfs2_ci_fully_checkpointed time_after(journal->j_trans_id, ci->ci_last_trans) + journal is empty, crash! For fixing, there are three solutions: 1> Partly revert commit da5e7c87827e8 For avoiding kernel crash, this make sense for us. We only concerned whether there has any non-system inode access before dlm init. The answer is NO. And all journal replay/recovery handling happen after dlm & journal init done. So this method is not graceful but workable. 2> Add osb->journal check in free inode routine (eg ocfs2_clear_inode) The fix code is special for mounting phase, but it will continue working after mounting stage. In another word, this method adds useless code in normal inode free flow. 3> Do directly free inode in mounting phase This method is brutal/complex and may introduce unsafe code, currently maintainer didn't like. At last, we chose method <1> and did partly reverted job. We reverted journal init codes, and kept cleanup codes flow. Link: https://lkml.kernel.org/r/20220424130952.2436-1-heming.zhao@suse.com Link: https://lkml.kernel.org/r/20220424130952.2436-2-heming.zhao@suse.com Fixes: da5e7c87827e8 ("ocfs2: cleanup journal init and shutdown") Signed-off-by: Heming Zhao <heming.zhao@suse.com> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> Cc: Mark Fasheh <mark@fasheh.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Junxiao Bi <junxiao.bi@oracle.com> Cc: Changwei Ge <gechangwei@live.cn> Cc: Gang He <ghe@suse.com> Cc: Jun Piao <piaojun@huawei.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- fs/ocfs2/inode.c | 4 ++-- fs/ocfs2/journal.c | 33 +++++++++++++++++++++++---------- fs/ocfs2/journal.h | 2 ++ fs/ocfs2/super.c | 15 +++++++++++++++ 4 files changed, 42 insertions(+), 12 deletions(-) --- a/fs/ocfs2/inode.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/inode.c @@ -125,6 +125,7 @@ struct inode *ocfs2_iget(struct ocfs2_su struct inode *inode = NULL; struct super_block *sb = osb->sb; struct ocfs2_find_inode_args args; + journal_t *journal = osb->journal->j_journal; trace_ocfs2_iget_begin((unsigned long long)blkno, flags, sysfile_type); @@ -171,11 +172,10 @@ struct inode *ocfs2_iget(struct ocfs2_su * part of the transaction - the inode could have been reclaimed and * now it is reread from disk. */ - if (osb->journal) { + if (journal) { transaction_t *transaction; tid_t tid; struct ocfs2_inode_info *oi = OCFS2_I(inode); - journal_t *journal = osb->journal->j_journal; read_lock(&journal->j_state_lock); if (journal->j_running_transaction) --- a/fs/ocfs2/journal.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/journal.c @@ -810,22 +810,20 @@ void ocfs2_set_journal_params(struct ocf write_unlock(&journal->j_state_lock); } -int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) +/* + * alloc & initialize skeleton for journal structure. + * ocfs2_journal_init() will make fs have journal ability. + */ +int ocfs2_journal_alloc(struct ocfs2_super *osb) { - int status = -1; - struct inode *inode = NULL; /* the journal inode */ - journal_t *j_journal = NULL; - struct ocfs2_journal *journal = NULL; - struct ocfs2_dinode *di = NULL; - struct buffer_head *bh = NULL; - int inode_lock = 0; + int status = 0; + struct ocfs2_journal *journal; - /* initialize our journal structure */ journal = kzalloc(sizeof(struct ocfs2_journal), GFP_KERNEL); if (!journal) { mlog(ML_ERROR, "unable to alloc journal\n"); status = -ENOMEM; - goto done; + goto bail; } osb->journal = journal; journal->j_osb = osb; @@ -839,6 +837,21 @@ int ocfs2_journal_init(struct ocfs2_supe INIT_WORK(&journal->j_recovery_work, ocfs2_complete_recovery); journal->j_state = OCFS2_JOURNAL_FREE; +bail: + return status; +} + +int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) +{ + int status = -1; + struct inode *inode = NULL; /* the journal inode */ + journal_t *j_journal = NULL; + struct ocfs2_journal *journal = osb->journal; + struct ocfs2_dinode *di = NULL; + struct buffer_head *bh = NULL; + int inode_lock = 0; + + BUG_ON(!journal); /* already have the inode for our journal */ inode = ocfs2_get_system_file_inode(osb, JOURNAL_SYSTEM_INODE, osb->slot_num); --- a/fs/ocfs2/journal.h~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/journal.h @@ -154,6 +154,7 @@ int ocfs2_compute_replay_slots(struct oc * Journal Control: * Initialize, Load, Shutdown, Wipe a journal. * + * ocfs2_journal_alloc - Initialize skeleton for journal structure. * ocfs2_journal_init - Initialize journal structures in the OSB. * ocfs2_journal_load - Load the given journal off disk. Replay it if * there's transactions still in there. @@ -167,6 +168,7 @@ int ocfs2_compute_replay_slots(struct oc * ocfs2_start_checkpoint - Kick the commit thread to do a checkpoint. */ void ocfs2_set_journal_params(struct ocfs2_super *osb); +int ocfs2_journal_alloc(struct ocfs2_super *osb); int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty); void ocfs2_journal_shutdown(struct ocfs2_super *osb); int ocfs2_journal_wipe(struct ocfs2_journal *journal, --- a/fs/ocfs2/super.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/super.c @@ -2195,6 +2195,15 @@ static int ocfs2_initialize_super(struct get_random_bytes(&osb->s_next_generation, sizeof(u32)); + /* + * FIXME + * This should be done in ocfs2_journal_init(), but any inode + * writes back operation will cause the filesystem to crash. + */ + status = ocfs2_journal_alloc(osb); + if (status < 0) + goto bail; + INIT_WORK(&osb->dquot_drop_work, ocfs2_drop_dquot_refs); init_llist_head(&osb->dquot_drop_list); @@ -2483,6 +2492,12 @@ static void ocfs2_delete_osb(struct ocfs kfree(osb->osb_orphan_wipes); kfree(osb->slot_recovery_generations); + /* FIXME + * This belongs in journal shutdown, but because we have to + * allocate osb->journal at the middle of ocfs2_initialize_super(), + * we free it here. + */ + kfree(osb->journal); kfree(osb->local_alloc_copy); kfree(osb->uuid_str); kfree(osb->vol_label); _ Patches currently in -mm which might be from ocfs2-devel@oss.oracle.com are _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org> To: mm-commits@vger.kernel.org, piaojun@huawei.com, mark@fasheh.com, junxiao.bi@oracle.com, joseph.qi@linux.alibaba.com, jlbec@evilplan.org, heming.zhao@suse.com, ghe@suse.com, gechangwei@live.cn, ocfs2-devel@oss.oracle.com, akpm@linux-foundation.org Subject: [merged mm-nonmm-stable] ocfs2-fix-mounting-crash-if-journal-is-not-alloced.patch removed from -mm tree Date: Mon, 09 May 2022 21:17:27 -0700 [thread overview] Message-ID: <20220510041728.165E3C385C7@smtp.kernel.org> (raw) The quilt patch titled Subject: ocfs2: fix mounting crash if journal is not alloced has been removed from the -mm tree. Its filename was ocfs2-fix-mounting-crash-if-journal-is-not-alloced.patch This patch was dropped because it was merged into the mm-nonmm-stable branch of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm ------------------------------------------------------ From: Heming Zhao via Ocfs2-devel <ocfs2-devel@oss.oracle.com> Subject: ocfs2: fix mounting crash if journal is not alloced Patch series "rewrite error handling during mounting stage". This patch (of 5): After commit da5e7c87827e8 ("ocfs2: cleanup journal init and shutdown"), journal init later than before, it makes NULL pointer access in free routine. Crash flow: ocfs2_fill_super + ocfs2_mount_volume | + ocfs2_dlm_init //fail & return, osb->journal is NULL. | + ... | + ocfs2_check_volume //no chance to init osb->journal | + ... + ocfs2_dismount_volume ocfs2_release_system_inodes ... evict ... ocfs2_clear_inode ocfs2_checkpoint_inode ocfs2_ci_fully_checkpointed time_after(journal->j_trans_id, ci->ci_last_trans) + journal is empty, crash! For fixing, there are three solutions: 1> Partly revert commit da5e7c87827e8 For avoiding kernel crash, this make sense for us. We only concerned whether there has any non-system inode access before dlm init. The answer is NO. And all journal replay/recovery handling happen after dlm & journal init done. So this method is not graceful but workable. 2> Add osb->journal check in free inode routine (eg ocfs2_clear_inode) The fix code is special for mounting phase, but it will continue working after mounting stage. In another word, this method adds useless code in normal inode free flow. 3> Do directly free inode in mounting phase This method is brutal/complex and may introduce unsafe code, currently maintainer didn't like. At last, we chose method <1> and did partly reverted job. We reverted journal init codes, and kept cleanup codes flow. Link: https://lkml.kernel.org/r/20220424130952.2436-1-heming.zhao@suse.com Link: https://lkml.kernel.org/r/20220424130952.2436-2-heming.zhao@suse.com Fixes: da5e7c87827e8 ("ocfs2: cleanup journal init and shutdown") Signed-off-by: Heming Zhao <heming.zhao@suse.com> Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com> Cc: Mark Fasheh <mark@fasheh.com> Cc: Joel Becker <jlbec@evilplan.org> Cc: Junxiao Bi <junxiao.bi@oracle.com> Cc: Changwei Ge <gechangwei@live.cn> Cc: Gang He <ghe@suse.com> Cc: Jun Piao <piaojun@huawei.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- fs/ocfs2/inode.c | 4 ++-- fs/ocfs2/journal.c | 33 +++++++++++++++++++++++---------- fs/ocfs2/journal.h | 2 ++ fs/ocfs2/super.c | 15 +++++++++++++++ 4 files changed, 42 insertions(+), 12 deletions(-) --- a/fs/ocfs2/inode.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/inode.c @@ -125,6 +125,7 @@ struct inode *ocfs2_iget(struct ocfs2_su struct inode *inode = NULL; struct super_block *sb = osb->sb; struct ocfs2_find_inode_args args; + journal_t *journal = osb->journal->j_journal; trace_ocfs2_iget_begin((unsigned long long)blkno, flags, sysfile_type); @@ -171,11 +172,10 @@ struct inode *ocfs2_iget(struct ocfs2_su * part of the transaction - the inode could have been reclaimed and * now it is reread from disk. */ - if (osb->journal) { + if (journal) { transaction_t *transaction; tid_t tid; struct ocfs2_inode_info *oi = OCFS2_I(inode); - journal_t *journal = osb->journal->j_journal; read_lock(&journal->j_state_lock); if (journal->j_running_transaction) --- a/fs/ocfs2/journal.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/journal.c @@ -810,22 +810,20 @@ void ocfs2_set_journal_params(struct ocf write_unlock(&journal->j_state_lock); } -int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) +/* + * alloc & initialize skeleton for journal structure. + * ocfs2_journal_init() will make fs have journal ability. + */ +int ocfs2_journal_alloc(struct ocfs2_super *osb) { - int status = -1; - struct inode *inode = NULL; /* the journal inode */ - journal_t *j_journal = NULL; - struct ocfs2_journal *journal = NULL; - struct ocfs2_dinode *di = NULL; - struct buffer_head *bh = NULL; - int inode_lock = 0; + int status = 0; + struct ocfs2_journal *journal; - /* initialize our journal structure */ journal = kzalloc(sizeof(struct ocfs2_journal), GFP_KERNEL); if (!journal) { mlog(ML_ERROR, "unable to alloc journal\n"); status = -ENOMEM; - goto done; + goto bail; } osb->journal = journal; journal->j_osb = osb; @@ -839,6 +837,21 @@ int ocfs2_journal_init(struct ocfs2_supe INIT_WORK(&journal->j_recovery_work, ocfs2_complete_recovery); journal->j_state = OCFS2_JOURNAL_FREE; +bail: + return status; +} + +int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty) +{ + int status = -1; + struct inode *inode = NULL; /* the journal inode */ + journal_t *j_journal = NULL; + struct ocfs2_journal *journal = osb->journal; + struct ocfs2_dinode *di = NULL; + struct buffer_head *bh = NULL; + int inode_lock = 0; + + BUG_ON(!journal); /* already have the inode for our journal */ inode = ocfs2_get_system_file_inode(osb, JOURNAL_SYSTEM_INODE, osb->slot_num); --- a/fs/ocfs2/journal.h~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/journal.h @@ -154,6 +154,7 @@ int ocfs2_compute_replay_slots(struct oc * Journal Control: * Initialize, Load, Shutdown, Wipe a journal. * + * ocfs2_journal_alloc - Initialize skeleton for journal structure. * ocfs2_journal_init - Initialize journal structures in the OSB. * ocfs2_journal_load - Load the given journal off disk. Replay it if * there's transactions still in there. @@ -167,6 +168,7 @@ int ocfs2_compute_replay_slots(struct oc * ocfs2_start_checkpoint - Kick the commit thread to do a checkpoint. */ void ocfs2_set_journal_params(struct ocfs2_super *osb); +int ocfs2_journal_alloc(struct ocfs2_super *osb); int ocfs2_journal_init(struct ocfs2_super *osb, int *dirty); void ocfs2_journal_shutdown(struct ocfs2_super *osb); int ocfs2_journal_wipe(struct ocfs2_journal *journal, --- a/fs/ocfs2/super.c~ocfs2-fix-mounting-crash-if-journal-is-not-alloced +++ a/fs/ocfs2/super.c @@ -2195,6 +2195,15 @@ static int ocfs2_initialize_super(struct get_random_bytes(&osb->s_next_generation, sizeof(u32)); + /* + * FIXME + * This should be done in ocfs2_journal_init(), but any inode + * writes back operation will cause the filesystem to crash. + */ + status = ocfs2_journal_alloc(osb); + if (status < 0) + goto bail; + INIT_WORK(&osb->dquot_drop_work, ocfs2_drop_dquot_refs); init_llist_head(&osb->dquot_drop_list); @@ -2483,6 +2492,12 @@ static void ocfs2_delete_osb(struct ocfs kfree(osb->osb_orphan_wipes); kfree(osb->slot_recovery_generations); + /* FIXME + * This belongs in journal shutdown, but because we have to + * allocate osb->journal at the middle of ocfs2_initialize_super(), + * we free it here. + */ + kfree(osb->journal); kfree(osb->local_alloc_copy); kfree(osb->uuid_str); kfree(osb->vol_label); _ Patches currently in -mm which might be from ocfs2-devel@oss.oracle.com are
next reply other threads:[~2022-05-10 4:17 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-10 4:17 Andrew Morton via Ocfs2-devel [this message] 2022-05-10 4:17 ` [merged mm-nonmm-stable] ocfs2-fix-mounting-crash-if-journal-is-not-alloced.patch removed from -mm tree Andrew Morton
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=20220510041728.165E3C385C7@smtp.kernel.org \ --to=ocfs2-devel@oss.oracle.com \ --cc=akpm@linux-foundation.org \ --cc=gechangwei@live.cn \ --cc=ghe@suse.com \ --cc=heming.zhao@suse.com \ --cc=jlbec@evilplan.org \ --cc=joseph.qi@linux.alibaba.com \ --cc=junxiao.bi@oracle.com \ --cc=mark@fasheh.com \ --cc=mm-commits@vger.kernel.org \ --cc=piaojun@huawei.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.