Re: [PATCH] ext4: Buffer Head Leak In mmp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mar 13, 2016, at 3:57 PM, Theodore Ts'o <tytso@xxxxxxx> wrote:
> 
> I've made the changes Andreas suggested, and ended up with this.  Andreas, PTAL?

Sorry, not sure what "PTAL" means... :-)

In any case, the patch looks good to me.

Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx.

> commit 0304688676bdfc8159e165313d71da19c118ba27
> Author: vikram.jadhav07 <vikramjadhavpucsd2007@xxxxxxxxx>
> Date:   Sun Mar 13 17:56:52 2016 -0400
> 
>    ext4: clean up error handling in the MMP support
> 
>    There is memory leak as both caller function kmmpd() and callee
>    read_mmp_block() not releasing bh_check  (i.e buffer_head).
>    Given patch fixes this problem.
> 
>    [ Additional changes suggested by Andreas Dilger -- TYT ]
> 
>    Signed-off-by: Jadhav Vikram <vikramjadhavpucsd2007@xxxxxxxxx>
>    Signed-off-by: Theodore Ts'o <tytso@xxxxxxx>
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 0a512aa..2444527 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -91,21 +91,22 @@ static int read_mmp_block(struct super_block *sb, struct buffer_head **bh,
> 	submit_bh(READ_SYNC | REQ_META | REQ_PRIO, *bh);
> 	wait_on_buffer(*bh);
> 	if (!buffer_uptodate(*bh)) {
> -		brelse(*bh);
> -		*bh = NULL;
> 		ret = -EIO;
> 		goto warn_exit;
> 	}
> -
> 	mmp = (struct mmp_struct *)((*bh)->b_data);
> -	if (le32_to_cpu(mmp->mmp_magic) != EXT4_MMP_MAGIC)
> +	if (le32_to_cpu(mmp->mmp_magic) != EXT4_MMP_MAGIC) {
> 		ret = -EFSCORRUPTED;
> -	else if (!ext4_mmp_csum_verify(sb, mmp))
> +		goto warn_exit;
> +	}
> +	if (!ext4_mmp_csum_verify(sb, mmp)) {
> 		ret = -EFSBADCRC;
> -	else
> -		return 0;
> -
> +		goto warn_exit;
> +	}
> +	return 0;
> warn_exit:
> +	brelse(*bh);
> +	*bh = NULL;
> 	ext4_warning(sb, "Error %d while reading MMP block %llu",
> 		     ret, mmp_block);
> 	return ret;
> @@ -181,15 +182,13 @@ static int kmmpd(void *data)
> 		    EXT4_FEATURE_INCOMPAT_MMP)) {
> 			ext4_warning(sb, "kmmpd being stopped since MMP feature"
> 				     " has been disabled.");
> -			EXT4_SB(sb)->s_mmp_tsk = NULL;
> -			goto failed;
> +			goto exit_thread;
> 		}
> 
> 		if (sb->s_flags & MS_RDONLY) {
> 			ext4_warning(sb, "kmmpd being stopped since filesystem "
> 				     "has been remounted as readonly.");
> -			EXT4_SB(sb)->s_mmp_tsk = NULL;
> -			goto failed;
> +			goto exit_thread;
> 		}
> 
> 		diff = jiffies - last_update_time;
> @@ -211,9 +210,7 @@ static int kmmpd(void *data)
> 			if (retval) {
> 				ext4_error(sb, "error reading MMP data: %d",
> 					   retval);
> -
> -				EXT4_SB(sb)->s_mmp_tsk = NULL;
> -				goto failed;
> +				goto exit_thread;
> 			}
> 
> 			mmp_check = (struct mmp_struct *)(bh_check->b_data);
> @@ -225,7 +222,9 @@ static int kmmpd(void *data)
> 					     "The filesystem seems to have been"
> 					     " multiply mounted.");
> 				ext4_error(sb, "abort");
> -				goto failed;
> +				put_bh(bh_check);
> +				retval = -EBUSY;
> +				goto exit_thread;
> 			}
> 			put_bh(bh_check);
> 		}
> @@ -248,7 +247,8 @@ static int kmmpd(void *data)
> 
> 	retval = write_mmp_block(sb, bh);
> 
> -failed:
> +exit_thread:
> +	EXT4_SB(sb)->s_mmp_tsk = NULL;
> 	kfree(data);
> 	brelse(bh);
> 	return retval;


Cheers, Andreas





Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail


[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux