Commit 618f0031 authored by Pavel Skripkin's avatar Pavel Skripkin Committed by Theodore Ts'o
Browse files

ext4: fix memory leak in ext4_fill_super 



static int kthread(void *_create) will return -ENOMEM
or -EINTR in case of internal failure or
kthread_stop() call happens before threadfn call.

To prevent fancy error checking and make code
more straightforward we moved all cleanup code out
of kmmpd threadfn.

Also, dropped struct mmpd_data at all. Now struct super_block
is a threadfn data and struct buffer_head embedded into
struct ext4_sb_info.

Reported-by: default avatar <syzbot+d9e482e303930fa4f6ff@syzkaller.appspotmail.com>
Signed-off-by: default avatarPavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/20210430185046.15742-1-paskripkin@gmail.com


Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent 1fc57ca5

fs/ext4/ext4.h

+4 −0
Original line number Diff line number Diff line
@@ -1488,6 +1488,7 @@ struct ext4_sb_info { 
	struct kobject s_kobj;

	struct completion s_kobj_unregister;

	struct super_block *s_sb;

	struct buffer_head *s_mmp_bh;



	/* Journaling */

	struct journal_s *s_journal;
@@ -3720,6 +3721,9 @@ extern struct ext4_io_end_vec *ext4_last_io_end_vec(ext4_io_end_t *io_end); 
/* mmp.c */

extern int ext4_multi_mount_protect(struct super_block *, ext4_fsblk_t);



/* mmp.c */

extern void ext4_stop_mmpd(struct ext4_sb_info *sbi);



/* verity.c */

extern const struct fsverity_operations ext4_verityops;

fs/ext4/mmp.c

+13 −15
Original line number Diff line number Diff line
@@ -127,9 +127,9 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp, 
 */

static int kmmpd(void *data)

{

	struct super_block *sb = ((struct mmpd_data *) data)->sb;

	struct buffer_head *bh = ((struct mmpd_data *) data)->bh;

	struct super_block *sb = (struct super_block *) data;

	struct ext4_super_block *es = EXT4_SB(sb)->s_es;

	struct buffer_head *bh = EXT4_SB(sb)->s_mmp_bh;

	struct mmp_struct *mmp;

	ext4_fsblk_t mmp_block;

	u32 seq = 0;
@@ -245,12 +245,18 @@ static int kmmpd(void *data) 
	retval = write_mmp_block(sb, bh);



exit_thread:

	EXT4_SB(sb)->s_mmp_tsk = NULL;

	kfree(data);

	brelse(bh);

	return retval;

}



void ext4_stop_mmpd(struct ext4_sb_info *sbi)

{

	if (sbi->s_mmp_tsk) {

		kthread_stop(sbi->s_mmp_tsk);

		brelse(sbi->s_mmp_bh);

		sbi->s_mmp_tsk = NULL;

	}

}



/*

 * Get a random new sequence number but make sure it is not greater than

 * EXT4_MMP_SEQ_MAX.
@@ -275,7 +281,6 @@ int ext4_multi_mount_protect(struct super_block *sb, 
	struct ext4_super_block *es = EXT4_SB(sb)->s_es;

	struct buffer_head *bh = NULL;

	struct mmp_struct *mmp = NULL;

	struct mmpd_data *mmpd_data;

	u32 seq;

	unsigned int mmp_check_interval = le16_to_cpu(es->s_mmp_update_interval);

	unsigned int wait_time = 0;
@@ -364,24 +369,17 @@ int ext4_multi_mount_protect(struct super_block *sb, 
		goto failed;

	}



	mmpd_data = kmalloc(sizeof(*mmpd_data), GFP_KERNEL);

	if (!mmpd_data) {

		ext4_warning(sb, "not enough memory for mmpd_data");

		goto failed;

	}

	mmpd_data->sb = sb;

	mmpd_data->bh = bh;

	EXT4_SB(sb)->s_mmp_bh = bh;



	/*

	 * Start a kernel thread to update the MMP block periodically.

	 */

	EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, mmpd_data, "kmmpd-%.*s",

	EXT4_SB(sb)->s_mmp_tsk = kthread_run(kmmpd, sb, "kmmpd-%.*s",

					     (int)sizeof(mmp->mmp_bdevname),

					     bdevname(bh->b_bdev,

						      mmp->mmp_bdevname));

	if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {

		EXT4_SB(sb)->s_mmp_tsk = NULL;

		kfree(mmpd_data);

		ext4_warning(sb, "Unable to create kmmpd thread for %s.",

			     sb->s_id);

		goto failed;

fs/ext4/super.c

+4 −6
Original line number Diff line number Diff line
@@ -1245,8 +1245,8 @@ static void ext4_put_super(struct super_block *sb) 
	ext4_xattr_destroy_cache(sbi->s_ea_block_cache);

	sbi->s_ea_block_cache = NULL;



	if (sbi->s_mmp_tsk)

		kthread_stop(sbi->s_mmp_tsk);

	ext4_stop_mmpd(sbi);



	brelse(sbi->s_sbh);

	sb->s_fs_info = NULL;

	/*
@@ -5186,8 +5186,7 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) 
failed_mount3:

	flush_work(&sbi->s_error_work);

	del_timer_sync(&sbi->s_err_report);

	if (sbi->s_mmp_tsk)

		kthread_stop(sbi->s_mmp_tsk);

	ext4_stop_mmpd(sbi);

failed_mount2:

	rcu_read_lock();

	group_desc = rcu_dereference(sbi->s_group_desc);
@@ -5989,8 +5988,7 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data) 
				 */

				ext4_mark_recovery_complete(sb, es);

			}

			if (sbi->s_mmp_tsk)

				kthread_stop(sbi->s_mmp_tsk);

			ext4_stop_mmpd(sbi);

		} else {

			/* Make sure we can mount this feature set readwrite */

			if (ext4_has_feature_readonly(sb) ||