| Message ID | 20200318061301.4320-1-riteshh@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ext4: Unregister sysfs path before destroying jbd2 journal | expand |
On Wed 18-03-20 11:43:01, Ritesh Harjani wrote: > Call ext4_unregister_sysfs(), before destroying jbd2 journal, > since below might cause, NULL pointer dereference issue. > This got reported with LTP tests. > > ext4_put_super() cat /sys/fs/ext4/loop2/journal_task > | ext4_attr_show(); > ext4_jbd2_journal_destroy(); | > | journal_task_show() > | | > | task_pid_vnr(NULL); > sbi->s_journal = NULL; > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> Yeah, makes sence. Thanks for the patch! You can add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/ext4/super.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 5dc65b7583cb..27ab130a40d1 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1024,6 +1024,13 @@ static void ext4_put_super(struct super_block *sb) > > destroy_workqueue(sbi->rsv_conversion_wq); > > + /* > + * Unregister sysfs before destroying jbd2 journal. > + * Since we could still access attr_journal_task attribute via sysfs > + * path which could have sbi->s_journal->j_task as NULL > + */ > + ext4_unregister_sysfs(sb); > + > if (sbi->s_journal) { > aborted = is_journal_aborted(sbi->s_journal); > err = jbd2_journal_destroy(sbi->s_journal); > @@ -1034,7 +1041,6 @@ static void ext4_put_super(struct super_block *sb) > } > } > > - ext4_unregister_sysfs(sb); > ext4_es_unregister_shrinker(sbi); > del_timer_sync(&sbi->s_err_report); > ext4_release_system_zone(sb); > -- > 2.21.0 >
On Wed, Mar 18, 2020 at 10:13:18AM +0100, Jan Kara wrote: > On Wed 18-03-20 11:43:01, Ritesh Harjani wrote: > > Call ext4_unregister_sysfs(), before destroying jbd2 journal, > > since below might cause, NULL pointer dereference issue. > > This got reported with LTP tests. > > > > ext4_put_super() cat /sys/fs/ext4/loop2/journal_task > > | ext4_attr_show(); > > ext4_jbd2_journal_destroy(); | > > | journal_task_show() > > | | > > | task_pid_vnr(NULL); > > sbi->s_journal = NULL; > > > > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> > > Yeah, makes sence. Thanks for the patch! You can add: > > Reviewed-by: Jan Kara <jack@suse.cz> Applied, thanks. - Ted
diff --git a/fs/ext4/super.c b/fs/ext4/super.c index 5dc65b7583cb..27ab130a40d1 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1024,6 +1024,13 @@ static void ext4_put_super(struct super_block *sb) destroy_workqueue(sbi->rsv_conversion_wq); + /* + * Unregister sysfs before destroying jbd2 journal. + * Since we could still access attr_journal_task attribute via sysfs + * path which could have sbi->s_journal->j_task as NULL + */ + ext4_unregister_sysfs(sb); + if (sbi->s_journal) { aborted = is_journal_aborted(sbi->s_journal); err = jbd2_journal_destroy(sbi->s_journal); @@ -1034,7 +1041,6 @@ static void ext4_put_super(struct super_block *sb) } } - ext4_unregister_sysfs(sb); ext4_es_unregister_shrinker(sbi); del_timer_sync(&sbi->s_err_report); ext4_release_system_zone(sb);
Call ext4_unregister_sysfs(), before destroying jbd2 journal, since below might cause, NULL pointer dereference issue. This got reported with LTP tests. ext4_put_super() cat /sys/fs/ext4/loop2/journal_task | ext4_attr_show(); ext4_jbd2_journal_destroy(); | | journal_task_show() | | | task_pid_vnr(NULL); sbi->s_journal = NULL; Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com> --- fs/ext4/super.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)