[CRIU] [PATCH v2] criu: correctly handle mixed-permission mapped files

[Jamie Liu]

Hi Andrew,

This turns out to be a very closely related problem, so I'll send out
a revision that fixes both issues, and update the test to test for
both (and address your comments on the test).

Thanks,
Jamie

On Thu, Apr 3, 2014 at 1:48 AM, Andrew Vagin <avagin at parallels.com> wrote:
> On Wed, Apr 02, 2014 at 05:52:56PM -0700, Jamie Liu wrote:
>> An mmaped file is opened O_RDONLY or O_RDWR depending on the permissions
>> on the first vma dump_task_mm() encounters mapping that file. This means
>> that if a file has multiple MAP_SHARED mappings, some of which are
>> read-only and some of which are read-write, and the first encountered
>> mapping happens to be read-only, the file will be opened with O_RDONLY
>> during restore, and mmap(PROT_WRITE) will fail with EACCES.
>>
>> To fix this, defer filemap open flags selection until restore, since
>> mapped files are opened per mapping.
>
> Hi Jamie,
>
> I have modified your test and it fails even with this patch.
>
> The idea of my changes is simple, I call mprotect after suspend/resume:
>
> open(xxx, O_RDWR)
> mmap(0, size, PROT_READ, MAP_SHARED, fd, 0);
> /* suspend/resume */
> mprotect(addr, size, PROT_READ | PROT_WRITE))
>
>
> Execute zdtm/live/static/maps_file_mixedprot
> ./maps_file_mixedprot --pidfile=maps_file_mixedprot.pid --outfile=maps_file_mixedprot.out --filename=maps_file_mixedprot.test
> Dump 29705
> Restore
> Check results 29705
> 12:41:42.588: 29705: ERR: maps_file_mixedprot.c:39: mprotect failed (errno = 13 (Permission denied))
> Test: zdtm/live/static/maps_file_mixedprot, Result: FAIL
> ==================================== ERROR ====================================
> Test: zdtm/live/static/maps_file_mixedprot, Namespace:
> Dump log   : /root/git/crtools/test/dump/maps_file_mixedprot/29705/1/dump.log
> --------------------------------- grep Error ---------------------------------
> ------------------------------------- END -------------------------------------
> Restore log: /root/git/crtools/test/dump/maps_file_mixedprot/29705/1/restore.log
> --------------------------------- grep Error ---------------------------------
> ------------------------------------- END -------------------------------------
> Output file: /root/git/crtools/test/zdtm/live/static/maps_file_mixedprot.out
> ------------------------------------------------------------------------------
> 12:41:42.588: 29705: ERR: maps_file_mixedprot.c:39: mprotect failed (errno = 13 (Permission denied))
> ------------------------------------- END -------------------------------------
> ================================= ERROR OVER =================================
>
>>
>> Signed-off-by: Jamie Liu <jamieliu at google.com>
>> ---
>>  cr-dump.c   | 6 +-----
>>  files-reg.c | 8 ++++++++
>>  2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/cr-dump.c b/cr-dump.c
>> index 0111115..f7945d9 100644
>> --- a/cr-dump.c
>> +++ b/cr-dump.c
>> @@ -15,7 +15,6 @@
>>  #include <sys/vfs.h>
>>
>>  #include <sys/sendfile.h>
>> -#include <sys/mman.h>
>>
>>  #include <sched.h>
>>  #include <sys/resource.h>
>> @@ -347,10 +346,7 @@ static int dump_filemap(pid_t pid, struct vma_area *vma_area,
>>       BUG_ON(!vma_area->st);
>>       p.stat = *vma_area->st;
>>
>> -     if ((vma->prot & PROT_WRITE) && vma_entry_is(vma, VMA_FILE_SHARED))
>> -             p.flags = O_RDWR;
>> -     else
>> -             p.flags = O_RDONLY;
>> +     /* Flags will be detected during restore in get_filemap_fd() */
>>
>>       if (fd_id_generate_special(&p.stat, &id))
>>               ret = dump_one_reg_file(vma_area->vm_file_fd, id, &p);
>> diff --git a/files-reg.c b/files-reg.c
>> index eccd04e..743b035 100644
>> --- a/files-reg.c
>> +++ b/files-reg.c
>> @@ -3,6 +3,7 @@
>>  #include <errno.h>
>>  #include <string.h>
>>  #include <sys/types.h>
>> +#include <sys/mman.h>
>>  #include <sys/stat.h>
>>  #include <sys/vfs.h>
>>  #include <ctype.h>
>> @@ -708,6 +709,8 @@ int open_reg_by_id(u32 id)
>>
>>  int get_filemap_fd(struct vma_area *vma)
>>  {
>> +     struct reg_file_info *rfi;
>> +
>>       /*
>>        * Thevma->fd should have been assigned in collect_filemap
>>        *
>> @@ -715,6 +718,11 @@ int get_filemap_fd(struct vma_area *vma)
>>        */
>>
>>       BUG_ON(vma->fd == NULL);
>> +     rfi = container_of(vma->fd, struct reg_file_info, d);
>> +     if ((vma->e->prot & PROT_WRITE) && vma_area_is(vma, VMA_FILE_SHARED))
>> +             rfi->rfe->flags = O_RDWR;
>> +     else
>> +             rfi->rfe->flags = O_RDONLY;
>>       return open_path(vma->fd, do_open_reg_noseek, NULL);
>>  }
>>
>> --
>> 1.9.1.423.g4596e3a
>>
>> _______________________________________________
>> CRIU mailing list
>> CRIU at openvz.org
>> https://lists.openvz.org/mailman/listinfo/criu