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 for Android: free password hash cracker in your pocket

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

Message-ID: <4E39338B.8020308@itwm.fraunhofer.de>
Date:	Wed, 03 Aug 2011 13:39:55 +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)

On 07/29/2011 08:25 PM, Andreas Dilger wrote:
> On 2011-07-29, at 11:44 AM, Bernd Schubert wrote:
>> 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)
>>> +{
>>> +	/* 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().
>
> One of the reasons for doing a direct open+seek+read here is because previous
> versions of e2fsprogs did not allow O_DIRECT reads via io_channel_read_blk64(),
> which is critical for the correct MMP functioning.  It cannot allow read from
> a cached block, either in libext2fs or in the VM, though a cached block on the
> disk is OK as long as it is visible to all hosts that are accessing it.

Yeah, I understand that it had to be O_DIRECT, but I didn't know about 
(previous) limitation of io_channel_read_blk64(). Maybe you could add a 
comment about it to the code about that, if you want to keep it? 
Something like
"/* We don't use io_channel_read_blk64() here to make sure blocks are 
really not cached */"?

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