[Ocfs2-test-devel] [PATCH 42/59] mmap-test: fix segment fault

[Junxiao Bi]

On 10/10/2015 03:09 PM, Eric Ren wrote:
> Hi Junxiao,
> 
> After learning the whole thing about mmap test, I found more issues.
> 
> 1. In my test, segment faults didn't happen. I did the following
> ====
> steps:
> ##
> eric at laptop:~> gcc mmap_test.c   ===> copy from ocfs2-test directly
> eric at laptop:~> dd if=/dev/zero of=test bs=4096 count=2
> 2+0 records in
> 2+0 records out
> 8192 bytes (8.2 kB) copied, 0.000187554 s, 43.7 MB/s
> eric at laptop:~> ls -sh test
> 8.0K test
> eric at laptop:~> ./a.out test
> file size: 8192         ===> add printf
> page size: 4096
> buf = 0x7fc277f15000, ptr = 0x7fc277f17000, size = 8192, offset = 0,
> remain = 4096
> tail of ptr: ""
> tail of page: "tail of page: "
> 5000, ptr = 0x7f000, ptr = 0x7fe = 8192, offset = 8192, offset 096
> 6
Maybe next page(address is $ptr) after the two mapping pages is also
mapped, so no segment fault.

> "
> ##
> 
> 2. How test suits use mmap_test
> ==============
> eric at laptop:~/ocfs2-test> vim programs/python_common/single_run-WIP.sh +711
>  699 run_mmap(){
> ...
>  725 
>  726   dd if=/dev/zero of=${workfile} bs=1M count=1024
>  727   mmap_test ${workfile} >${outlog} 2>&1
>  728   RC=$?
> ...
> }
> 
> This way, workfile=1G is always multiple size of page size(4096).
> I think bs=1M is bad.
> 
> 3. What mmap_test want to do
> ===========================
> eric at laptop:~> cd ocfs2-test/
> eric at laptop:~/ocfs2-test> vim programs/mmap_test/mmap_test.c
> ...
> 75     buf = mmap(NULL, stat_buf.st_size, PROT_READ, MAP_SHARED, fd, 0);
>  76     if (buf == MAP_FAILED)
>  77     {
>  78         perror("MMap");
>  79         return 1;
>  80     }
>  81 
>  82     offset = stat_buf.st_size % page_size;
>  83     ptr = buf + (stat_buf.st_size - offset);
>  84     remain = page_size - offset;
>  85 
>  86     fprintf(stdout, "buf = %p, ptr = %p, size = %lu, offset = %d, remain = %d\n",
>  87             buf, ptr, stat_buf.st_size, offset, remain);
>  88 
>  89     fprintf(stdout, "tail of ptr: \"");
>  90     fwrite(ptr, sizeof(char), offset, stdout);
>  91     fprintf(stdout, "\"\n");
>  92 
>  93     fprintf(stdout, "tail of page: \"");
>  94     fwrite(ptr + offset, sizeof(char), remain, stdout);
>  95     fprintf(stdout, "\"\n");
> ...
> 
> Looks like it want output the front part of the last mapping page, and the remaining of
> the last mapping page, right?
> 
> I think "tail of ptr" should be "head of last mapping page", and "tail of page" should
> be "tail of last mapping page".
I think "tail of ptr" may want to test read from the last char of the
mapped area and "tail of page" want to test read from the last char of
the last mapping page but out of the mapped area. So we should make the
test file size not page aligned.
> 
> But it didn't handle the case when offset==0. 
Yes. It even didn't work well when offset != 0. The following fwrite
will read from next unmapped page.

fwrite(ptr + offset, sizeof(char), remain, stdout);

> 
> 4. How to fix
> =============
> Frist,this line of code:
> ptr = buf + (stat_buf.st_size - offset);
> should be changed to
> ptr = buf + (stat_buf.st_size - offset -1);
This looks OK. With this, ptr will not read out of scope.
> 
> Then, when offset==0, the rest of code actually doesn't make any sense. Could we just
> return, or output the last whole page?
Yes, for this case, the two fwrite are doing the same thing, but i think
no need to check and skip the second one since no bad effect.

Thanks,
Junxiao.
> 
> Thanks,
> Eric
> 
> On Mon, Sep 14, 2015 at 10:44:28AM +0800, Junxiao Bi wrote: 
>> When test file size is multiple of page size, there will be write out of
>> mmap range and cause segment fault.
>>
>> Signed-off-by: Junxiao Bi <junxiao.bi at oracle.com>
>> ---
>>  programs/mmap_test/mmap_test.c |    8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/programs/mmap_test/mmap_test.c b/programs/mmap_test/mmap_test.c
>> index 9edbb58..711bb81 100644
>> --- a/programs/mmap_test/mmap_test.c
>> +++ b/programs/mmap_test/mmap_test.c
>> @@ -80,6 +80,14 @@ int main(int argc, char *argv[])
>>      }
>>  
>>      offset = stat_buf.st_size % page_size;
>> +
>> +    /* fix offset if the file size is multiple of page size */
>> +    if (offset == 0)
>> +	    offset = page_size;
>> +
>> +    if (offset > stat_buf.st_size)
>> +	    offset = stat_buf.st_size;
>> +
>>      ptr = buf + (stat_buf.st_size - offset);
>>      remain = page_size - offset;
>>  
>> -- 
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> Ocfs2-test-devel mailing list
>> Ocfs2-test-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/ocfs2-test-devel
>>