linux-ext4 - Re: [PATCH] libext2fs: Always swab MMP block on BE machines

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]

Date:	Fri, 30 Sep 2011 15:09:53 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	djwong@...ibm.com
Cc:	"Theodore Ts'o" <tytso@....edu>,
	linux-ext4 <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH] libext2fs: Always swab MMP block on BE machines

On 2011-09-30, at 1:41 PM, Darrick J. Wong wrote:
> The MMP code in libext2fs tries to gate MMP block swab'ing with this test:
> 
> #ifdef EXT2FS_ENABLE_SWAPFS
> if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC))

We've never run Lustre on big-endian servers (clients generally work),
so it's possible this has been in our patch for years.

Thanks for the catch.

> However, EXT2FS_ENABLE_SWAPFS never seems to be defined anywhere (all possible
> #define's were cleaned out of e2fsprogs ages ago), and even if any still
> existed, the field fs->super->s_magic is always in host byteorder, so the test
> always fails.  So, we can change the #ifdef to WORDS_BIGENDIAN (which is
> conditionally defined on BE platforms) and get rid of the broken if test.
> 
> (This fix came up while testing the metadata checksumming patchset)
> 
> Signed-off-by: Darrick J. Wong <djwong@...ibm.com>
> ---
> 
> lib/ext2fs/mmp.c |   15 ++++++---------
> 1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/ext2fs/mmp.c b/lib/ext2fs/mmp.c
> index ed6ee42..91f4fb2 100644
> --- a/lib/ext2fs/mmp.c
> +++ b/lib/ext2fs/mmp.c
> @@ -91,9 +91,8 @@ errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
> 	}
> 
> 	mmp_cmp = fs->mmp_cmp;
> -#ifdef EXT2FS_ENABLE_SWAPFS
> -	if (fs->flags & EXT2_FLAG_SWAP_BYTES)
> -		ext2fs_swap_mmp(mmp_cmp);
> +#ifdef WORDS_BIGENDIAN
> +	ext2fs_swap_mmp(mmp_cmp);
> #endif
> 
> 	if (buf != NULL && buf != fs->mmp_cmp)
> @@ -122,18 +121,16 @@ errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf)
> 	    fs->super->s_mmp_block > ext2fs_blocks_count(fs->super))
> 		return EXT2_ET_MMP_BAD_BLOCK;
> 
> -#ifdef EXT2FS_ENABLE_SWAPFS
> -	if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC))
> -		ext2fs_swap_mmp(mmp_s);
> +#ifdef WORDS_BIGENDIAN
> +	ext2fs_swap_mmp(mmp_s);
> #endif
> 
> 	/* I was tempted to make this use O_DIRECT and the mmp_fd, but
> 	 * this caused no end of grief, while leaving it as-is works. */
> 	retval = io_channel_write_blk64(fs->io, mmp_blk, -fs->blocksize, buf);
> 
> -#ifdef EXT2FS_ENABLE_SWAPFS
> -	if (fs->super->s_magic == ext2fs_swab16(EXT2_SUPER_MAGIC))
> -		ext2fs_swap_mmp(mmp_s);
> +#ifdef WORDS_BIGENDIAN
> +	ext2fs_swap_mmp(mmp_s);
> #endif
> 
> 	/* Make sure the block gets to disk quickly */
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html