linux-ext4 - Re: [PATCH] ext4: don't assume that mmp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D5CCD904-C596-4C05-B665-D28C63844D12@dilger.ca>
Date:   Sun, 26 Jan 2020 15:06:36 -0700
From:   Andreas Dilger <adilger@...ger.ca>
To:     tytso@....edu
Cc:     linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext4: don't assume that mmp_nodename/bdevname have NUL

On Jan 26, 2020, at 3:03 PM, Andreas Dilger <adilger@...ger.ca> wrote:
> 
> Don't assume that the mmp_nodename and mmp_bdevname strings are NUL
> terminated, since they are filled in by snprintf(), which is not
> guaranteed to do so.
> 
> Signed-off-by: Andreas Dilger <adilger@...ger.ca>
> ---

NB: this is v2 of the patch, which fixes the checkpatch warnings.

Ted, do you also want an ext4 patch with EXT4_LEN_STR() and a change of these
char strings to __u8, along with similar changes to other non-NUL-terminated
strings in the superblock, as was done for e2fsprogs?

Cheers, Andreas

> fs/ext4/mmp.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ext4/mmp.c b/fs/ext4/mmp.c
> index 2305b43..9d00e0d 100644
> --- a/fs/ext4/mmp.c
> +++ b/fs/ext4/mmp.c
> @@ -120,10 +120,10 @@ void __dump_mmp_msg(struct super_block *sb, struct mmp_struct *mmp,
> {
> 	__ext4_warning(sb, function, line, "%s", msg);
> 	__ext4_warning(sb, function, line,
> -		       "MMP failure info: last update time: %llu, last update "
> -		       "node: %s, last update device: %s",
> -		       (long long unsigned int) le64_to_cpu(mmp->mmp_time),
> -		       mmp->mmp_nodename, mmp->mmp_bdevname);
> +		       "MMP failure info: last update time: %llu, last update node: %.*s, last update device: %.*s",
> +		       (unsigned long long)le64_to_cpu(mmp->mmp_time),
> +		       (int)sizeof(mmp->mmp_nodename), mmp->mmp_nodename,
> +		       (int)sizeof(mmp->mmp_bdevname), mmp->mmp_bdevname);
> }
> 
> /*
> @@ -154,6 +154,7 @@ static int kmmpd(void *data)
> 	mmp_check_interval = max(EXT4_MMP_CHECK_MULT * mmp_update_interval,
> 				 EXT4_MMP_MIN_CHECK_INTERVAL);
> 	mmp->mmp_check_interval = cpu_to_le16(mmp_check_interval);
> +	BUILD_BUG_ON(sizeof(mmp->mmp_bdevname) < BDEVNAME_SIZE);
> 	bdevname(bh->b_bdev, mmp->mmp_bdevname);
> 
> 	memcpy(mmp->mmp_nodename, init_utsname()->nodename,
> @@ -375,7 +376,8 @@ int ext4_multi_mount_protect(struct super_block *sb,
> 	/*
> 	 * 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, mmpd_data, "kmmpd-%.*s",
> +					     (int)sizeof(mmp->mmp_bdevname),
> 					     bdevname(bh->b_bdev,
> 						      mmp->mmp_bdevname));
> 	if (IS_ERR(EXT4_SB(sb)->s_mmp_tsk)) {
> --
> 1.8.0
> 


Cheers, Andreas






Download attachment "signature.asc" of type "application/pgp-signature" (874 bytes)