linux-ext4 - Re: [PATCH] ext4:Fix a bug in ext4_ext_fiemap

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTikDiNP40x=CXKUYEyObtd+hX09cJ0A3H0_5dOaX@mail.gmail.com>
Date:	Thu, 24 Feb 2011 08:37:08 +0800
From:	Yongqiang Yang <xiaoqiangnk@...il.com>
To:	Andreas Dilger <adilger@...ger.ca>
Cc:	linux-ext4@...r.kernel.org, sandeen@...hat.com
Subject: Re: [PATCH] ext4:Fix a bug in ext4_ext_fiemap_cb().

On Thu, Feb 24, 2011 at 7:35 AM, Andreas Dilger <adilger@...ger.ca> wrote:
> On 2011-02-23, at 8:59 AM, Yongqiang Yang wrote:
>> @@ -3788,17 +3788,27 @@ static int ext4_ext_fiemap_cb(struct inode *inode,
>> -     if (newex->ec_start == 0) {
>> +     if (!newex->ec_start) {
>
> (style) the original code is actually correct.  ec_start is not a boolean, so comparing it == 0 is actually the right thing to do.
>
>> @@ -3807,8 +3817,13 @@ static int ext4_ext_fiemap_cb(struct inode *inode, struct ext4_ext_path *path,
>>               if (!bh)
>>                       return EXT_CONTINUE;
>>
>> +             /* Assume block-size equals page-size. */
>
> This is not a valid assumption.
>
>>               if (buffer_delay(bh)) {
>>                       flags |= FIEMAP_EXTENT_DELALLOC;
>> +                     if (page->index > offset) {
>> +                             logical =  ((__u64)page->index << PAGE_SHIFT);
>> +                             newex->ec_block = logical >> blksize_bits;
>> +                     }
>
> So, this assumes that the entire unmapped extent is described by the first page, but doesn't actually check whether all of the pages exist.  For the purpose of cp it might be OK, since at worst it means that cp will be reading from a hole in the source file.  However, I wonder if other applications will depend on the allocated extent being more accurate?
>
>> +EXPORT_SYMBOL(find_get_pages);
>
> Eric had also suggested pagevec_lookup_tag(), which is already exported.
Yes, we can use this function.  I will make a new patch.
>
> Cheers, Andreas
>
>
>
>
>
>



-- 
Best Wishes
Yongqiang Yang
--
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