linux-ext4 - Re: [PATCH v2] ext4:Let ext4_ext_fiemap_cb() handle blocks before request range correctly.

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

Date:	Fri, 13 May 2011 11:40:02 +0800
From:	Yongqiang Yang <xiaoqiangnk@...il.com>
To:	Lukas Czerner <lczerner@...hat.com>
Cc:	Eric Sandeen <sandeen@...deen.net>, josef@...hat.com,
	linux-ext4@...r.kernel.org, tytso@....edu
Subject: Re: [PATCH v2] ext4:Let ext4_ext_fiemap_cb() handle blocks before
 request range correctly.

On Tue, May 10, 2011 at 11:08 PM, Yongqiang Yang <xiaoqiangnk@...il.com> wrote:
> On Tue, May 10, 2011 at 10:35 PM, Lukas Czerner <lczerner@...hat.com> wrote:
>> On Tue, 10 May 2011, Yongqiang Yang wrote:
>>
>>> v1->v2:
>>>   Add more specific description.
>>>
>>> To get delayed-extent information, ext4_ext_fiemap_cb() lookups pagecache,
>>> it thus collects information starting from a page's head block.
>>> If blockszie < pagesize, the beginning blocks of a page may lie before the
>>> range.  ext4_ext_fiemap_cb() should proceed ignoring them, because they
>>> has been handled before.
>>
>> Thanks for the description, but I have one question below.
>>
>>>
>>> Reported-by: Amir Goldstein <amir73il@...rs.sf.net>
>>> Signed-off-by: Yongqiang Yang <xiaoqiangnk@...il.com>
>>> ---
>>>  fs/ext4/extents.c |    7 ++++++-
>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
>>> index e363f21..ec37109 100644
>>> --- a/fs/ext4/extents.c
>>> +++ b/fs/ext4/extents.c
>>> @@ -3718,9 +3718,14 @@ out:
>>>
>>>                       bh = head;
>>>                       do {
>>> +                             if (end < newex->ec_block)
>>> +                                     /* The buffer is not in
>>> +                                      * the request range.
>>> +                                      */
>>> +                                     continue;
>> So if the condition is true, then we might leave the loop, because (bh ==
>> head) in the first iteration, is that desired behavior? Also if we hit
>> this condition in other than first iteration we will spin forever. I do
>> not think this is right. How did you tested this case ?
> You are right.  Maybe the patch sent out is not same as the patch with
> which I tested.  What a stupid error!
> I am not sure what happened now.  I will check the patch with which I
> tested tomorrow.
>
> My working tree passed xfstests 225 in both 1k blocksize and 4k
> blocksize cases.
I checked the code I tested with.  Indeed I tested with this ugly
patch and xfstests 225 passed.  I tested again and results were same '
PASSED'.

I had a look into xfstests 225 and found that there are problems in
xfstests 225 so it can not find the bug induced by this ugly patch.
It seems that current xfstests 225 has a lot of problems.

I am fixing xfstests 225 to enable it find this bug.

Yongqiang.

>
> Sorry!
>
> Yongqiang.
>>
>> Thanks!
>> -Lukas
>>
>>>                               if (buffer_mapped(bh)) {
>>>                                       /* get the 1st mapped buffer. */
>>> -                                     if (end > newex->ec_block +
>>> +                                     if (end >= newex->ec_block +
>>>                                               newex->ec_len)
>>>                                               /* The buffer is out of
>>>                                                * the request range.
>>>
>>
>
>
>
> --
> Best Wishes
> Yongqiang Yang
>



-- 
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