linux-kernel - Re: [PATCH v2 1/2] ext4: Include forgotten start block on fallocate insert range

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]

Message-ID: <CAJrWOzAffA6_7wvA0OQXB4kwkjQvfazw_AE7Jo=Ohg=KPHxZ3g@mail.gmail.com>
Date:   Mon, 9 Jan 2017 13:36:34 +0100
From:   Roman Penyaev <roman.penyaev@...fitbricks.com>
To:     "Theodore Ts'o" <tytso@....edu>,
        Roman Pen <roman.penyaev@...fitbricks.com>,
        Namjae Jeon <namjae.jeon@...sung.com>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] ext4: Include forgotten start block on fallocate
 insert range

On Sat, Jan 7, 2017 at 10:22 PM, Theodore Ts'o <tytso@....edu> wrote:
> On Fri, Jan 06, 2017 at 09:26:00PM +0100, Roman Pen wrote:
>> While doing 'insert range' start block should be also shifted right.
>> The bug can be easily reproduced by the following test:
>>
>>     ptr = malloc(4096);
>>     assert(ptr);
>>
>>     fd = open("./ext4.file", O_CREAT | O_TRUNC | O_RDWR, 0600);
>>     assert(fd >= 0);
>>
>>     rc = fallocate(fd, 0, 0, 8192);
>>     assert(rc == 0);
>>     for (i = 0; i < 2048; i++)
>>             *((unsigned short *)ptr + i) = 0xbeef;
>>     rc = pwrite(fd, ptr, 4096, 0);
>>     assert(rc == 4096);
>>     rc = pwrite(fd, ptr, 4096, 4096);
>>     assert(rc == 4096);
>>
>>     for (block = 2; block < 1000; block++) {
>>             rc = fallocate(fd, FALLOC_FL_INSERT_RANGE, 4096, 4096);
>>             assert(rc == 0);
>>
>>             for (i = 0; i < 2048; i++)
>>                     *((unsigned short *)ptr + i) = block;
>>
>>             rc = pwrite(fd, ptr, 4096, 4096);
>>             assert(rc == 4096);
>>     }
>>
>> Because start block is not included in the range the hole appears at
>> the wrong offset (just after the desired offset) and the following
>> pwrite() overwrites already existent block, keeping hole untouched.
>>
>> Simple way to verify wrong behaviour is to check zeroed blocks after
>> the test:
>>
>>    $ hexdump ./ext4.file | grep '0000 0000'
>>
>> The root cause of the bug is a wrong range (start, stop], where start
>> should be inclusive, i.e. [start, stop].
>>
>> This patch fixes the problem by including start into the range.  But
>> not to break left shift (range collapse) stop points to the beginning
>> of the a block, not to the end.
>>
>> The other not obvious change is an iterator check on validness in a
>> main loop.  Because iterator is unsigned the following corner case
>> should be considered with care: insert a block at 0 offset, when stop
>> variables overflows and never becomes less than start, which is 0.
>> To handle this special case iterator is set to NULL to indicate that
>> end of the loop is reached.
>>
>> Signed-off-by: Roman Pen <roman.penyaev@...fitbricks.com>
>
> Thanks, applied.
>

Could you please provide with the SHA1 of the patch in your branch?
I want to make an exact reference in a new test of the xfstest which
covers that bug.

--
Roman