linux-ext4 - Re: [PATCH 1/2] ext2fs: add multi-mount protection (INCOMPAT

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, 29 Jul 2011 19:44:43 +0200
From:	Bernd Schubert <bernd.schubert@...m.fraunhofer.de>
To:	Andreas Dilger <adilger@...mcloud.com>
CC:	tytso@....edu, linux-ext4@...r.kernel.org,
	Johann Lombardi <johann@...mcloud.com>
Subject: Re: [PATCH 1/2] ext2fs: add multi-mount protection (INCOMPAT_MMP)

Andreas, Johann,

thanks a lot for sending those patches! From a previous review I still 
remember ext2fs_mmp_read(), see below.
I will look through the other parts later on.


> +errcode_t ext2fs_mmp_read(ext2_filsys fs, blk64_t mmp_blk, void *buf)
> +{
> +	struct mmp_struct *mmp_cmp;
> +	errcode_t retval = 0;
> +
> +	if ((mmp_blk<= fs->super->s_first_data_block) ||
> +	    (mmp_blk>= fs->super->s_blocks_count))
> +		return EXT2_ET_MMP_BAD_BLOCK;
> +
> +	if (fs->mmp_cmp == NULL) {
> +		/* O_DIRECT in linux 2.4: page aligned
> +		 * O_DIRECT in linux 2.6: sector aligned
> +		 * A filesystem cannot be created with blocksize<  sector size,
> +		 * or with blocksize>  page_size. */
> +		int bufsize = fs->blocksize;
> +
> +		if (bufsize<  mmp_pagesize())
> +			bufsize = mmp_pagesize();
> +		retval = ext2fs_get_memalign(bufsize, bufsize,&fs->mmp_cmp);
> +		if (retval)
> +			return retval;
> +	}
> +
> +	/* ext2fs_open reserves fd0,1,2 to avoid stdio collision */
> +	if (fs->mmp_fd<= 0) {
> +		fs->mmp_fd = open(fs->device_name, O_RDWR | O_DIRECT);
> +		if (fs->mmp_fd<  0) {
> +			retval = EXT2_ET_MMP_OPEN_DIRECT;
> +			goto out;
> +		}
> +	}
> +
> +	if (ext2fs_llseek(fs->mmp_fd, mmp_blk * fs->blocksize, SEEK_SET) !=
> +	    mmp_blk * fs->blocksize) {
> +		retval = EXT2_ET_LLSEEK_FAILED;
> +		goto out;
> +	}
> +
> +	if (read(fs->mmp_fd, fs->mmp_cmp, fs->blocksize) != fs->blocksize) {
> +		retval = EXT2_ET_SHORT_READ;
> +		goto out;
> +	}

While ext2fs_llseek() and read() works fine, I still wonder why the code 
does not use io_channel_read_blk64() here, similarly as 
io_channel_write_blk64() write in ext2fs_mmp_write().

> +
> +	mmp_cmp = fs->mmp_cmp;
> +#ifdef EXT2FS_ENABLE_SWAPFS
> +	if (fs->flags&  EXT2_FLAG_SWAP_BYTES)
> +		ext2fs_swap_mmp(mmp_cmp);
> +#endif
> +
> +	if (buf != NULL&&  buf != fs->mmp_cmp)
> +		memcpy(buf, fs->mmp_cmp, fs->blocksize);
> +
> +	if (mmp_cmp->mmp_magic != EXT4_MMP_MAGIC) {
> +		retval = EXT2_ET_MMP_MAGIC_INVALID;
> +		goto out;
> +	}
> +
> +out:
> +	return retval;
> +}
> +
> +errcode_t ext2fs_mmp_write(ext2_filsys fs, blk64_t mmp_blk, void *buf)
> +{
> +	struct mmp_struct *mmp_s = buf;
> +	struct timeval tv;
> +	errcode_t retval = 0;
> +
> +	gettimeofday(&tv, 0);
> +	mmp_s->mmp_time = tv.tv_sec;
> +	fs->mmp_last_written = tv.tv_sec;
> +
> +	if (fs->super->s_mmp_block<  fs->super->s_first_data_block ||
> +	    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);
> +#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);
> +#endif
> +
> +	/* Make sure the block gets to disk quickly */
> +	io_channel_flush(fs->io);
> +	return retval;
> +}


Thanks,
Bernd
--
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