[LTP] [PATCH 1/2] ltp-aiodio/dio_sparse: Fix write in dio_sparse()

[Guangwen Feng]

Hi!

On 01/25/2017 12:33 PM, Guangwen Feng wrote:
> Hi Cyril,
> 
> Thanks for your review.
> 
> On 01/24/2017 09:40 PM, Cyril Hrubis wrote:
>> Hi!
>>> Current code looks buggy because when the offset is greater than
>>> or equal to the filesize, it will never happen to do the write
>>> in the loop, as a result, ADSP080..ADSP087 do not work actually.
>>> Fix it by making sure that we do write writesize bytes.
>>>
>>> Signed-off-by: Guangwen Feng <fenggw-fnst@cn.fujitsu.com>
>>> ---
>>>  testcases/kernel/io/ltp-aiodio/dio_sparse.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> index 41b9929..3828ed7 100644
>>> --- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> +++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
>>> @@ -78,7 +78,7 @@ int dio_sparse(char *filename, int align, int writesize, int filesize, int offse
>>>  
>>>  	memset(bufptr, 0, writesize);
>>>  	lseek(fd, offset, SEEK_SET);
>>> -	for (i = offset; i < filesize;) {
>>> +	for (i = 0; i < filesize;) {
>>>  		if ((w = write(fd, bufptr, writesize)) != writesize) {
>>>  			tst_resm(TBROK | TERRNO, "write() returned %d", w);
>>>  			close(fd);
>>
>> Hmm, it looks to me like the code actually does what it should have.
>> Since we pass a filesize and offset so the test should actually write
>> filesize - offset bytes. As far as I can tell the bug here is that the
>> test is not checking that filesize > offset before it starts.
> 
> Sorry, but shouldn't we write the whole "writesize" from the "offset"?
> ADSP080 ADSP081 ADSP082 ADSP083 did reproduce the BUG via above modification...

I think we pass an "offset" is to make sure writing from the file offset,
but the actual write size should be just the "writesize" argument.

As the second case of kernel commit 9ecd10b says:
    2. Direct writes starting from or beyong i_size (not inside i_size)
       also could trigger block allocation and expose stale data.  For
       example, consider a sparse file with i_size of 2k, and a write to
       offset 2k or 3k into the file, with a filesystem block size of 4k.
       (Thanks to Jeff Moyer for pointing this case out in his review.)


Best Regards,
Guangwen Feng

> 
> Best Regards,
> Guangwen Feng