[CRIU] [PATCH] proc_parse: fix vma file open mode recognition
Pavel Emelyanov
xemul at virtuozzo.com
Tue Jul 5 05:19:34 PDT 2016
On 07/05/2016 02:18 PM, Pavel Emelyanov wrote:
> On 07/04/2016 09:59 PM, Stanislav Kinsburskiy wrote:
>>
>> 4 июля 2016 г. 20:05 пользователь Pavel Emelianov <xemul at virtuozzo.com> написал:
>>>
>>> On 06/30/2016 07:24 PM, Stanislav Kinsburskiy wrote:
>>>> The correct place to get fd open flags for file mappings is
>>>> /proc/<pid>/map_files.
>>>> An attempt tp speculate on "shared" and "maywrite" bits doesn't garantee,
>>>> that file will be opened with correct permissions on restore.
>>>> Here is an example:
>>>>
>>>> Process mapping (read/write):
>>>>
>>>> # cat /proc/481943/maps | grep 7f7108077000-7f7108078000
>>>> 7f7108077000-7f7108078000 rw-p 00001000 00:35 7 <snip>
>>>>
>>>> 1) Before suspend:
>>>>
>>>> # ls -l /proc/481427/map_files/7f7108077000-7f7108078000
>>>> lrw------- <snip> /proc/481427/map_files/7f7108077000-7f7108078000 -> <snip>
>>>>
>>>> 2) After restore:
>>>>
>>>> # ls -l /proc/481943/map_files/7f7108077000-7f7108078000
>>>> lr-------- <snip> /proc/481943/map_files/7f7108077000-7f7108078000 -> <snip>
>>>>
>>>> Write bit is lost.
BTW, can you explain in more details why messing with shared/maywrite doesn't
give us confidence if fdflags?
>>>> This patch set vma->e->fdflags as /proc/<pid>/map_files/<vma> open mode.
>>>>
>>>> Signed-off-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
>>>> ---
>>>> criu/proc_parse.c | 51 ++++++++++++++++++++++++++++++++++-----------------
>>>> criu/util.c | 3 ++-
>>>> 2 files changed, 36 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
>>>> index 5210226..8e36916 100644
>>>> --- a/criu/proc_parse.c
>>>> +++ b/criu/proc_parse.c
>>>> @@ -82,8 +82,6 @@ static bool is_vma_range_fmt(char *line)
>>>> static int parse_vmflags(char *buf, struct vma_area *vma_area)
>>>> {
>>>> char *tok;
>>>> - bool shared = false;
>>>> - bool maywrite = false;
>>>>
>>>> if (!buf[0])
>>>> return 0;
>>>> @@ -95,12 +93,6 @@ static int parse_vmflags(char *buf, struct vma_area *vma_area)
>>>> #define _vmflag_match(_t, _s) (_t[0] == _s[0] && _t[1] == _s[1])
>>>>
>>>> do {
>>>> - /* open() block */
>>>> - if (_vmflag_match(tok, "sh"))
>>>> - shared = true;
>>>> - else if (_vmflag_match(tok, "mw"))
>>>> - maywrite = true;
>>>> -
>>>> /* mmap() block */
>>>> if (_vmflag_match(tok, "gd"))
>>>> vma_area->e->flags |= MAP_GROWSDOWN;
>>>> @@ -144,12 +136,6 @@ static int parse_vmflags(char *buf, struct vma_area *vma_area)
>>>>
>>>> #undef _vmflag_match
>>>>
>>>> - if (shared && maywrite)
>>>> - vma_area->e->fdflags = O_RDWR;
>>>> - else
>>>> - vma_area->e->fdflags = O_RDONLY;
>>>> - vma_area->e->has_fdflags = true;
>>>> -
>>>> if (vma_area->e->madv)
>>>> vma_area->e->has_madv = true;
>>>>
>>>> @@ -175,12 +161,46 @@ static inline int vfi_equal(struct vma_file_info *a, struct vma_file_info *b)
>>>> (a->dev_min ^ b->dev_min)) == 0;
>>>> }
>>>>
>>>> +static int vma_get_mapfile_flags(struct vma_area *vma, DIR *mfd, char *path)
>>>> +{
>>>> + struct stat stat;
>>>> +
>>>> + if (fstatat(dirfd(mfd), path, &stat, AT_SYMLINK_NOFOLLOW) < 0) {
>>>> + if (errno == ENOENT) {
>>>> + /* Just mapping w/o map_files link */
>>>> + return 0;
>>>> + }
>>>> + pr_perror("Failed fstatat on map %"PRIx64"", vma->e->start);
>>>> + return -1;
>>>> + }
>>>> +
>>>> + switch(stat.st_mode & 0600) {
>>>> + case 0200:
>>>> + vma->e->fdflags = O_WRONLY;
>>>> + break;
>>>> + case 0400:
>>>> + vma->e->fdflags = O_RDONLY;
>>>> + break;
>>>> + case 0600:
>>>> + vma->e->fdflags = O_RDWR;
>>>> + break;
>>>> + }
>>>> + vma->e->has_fdflags = true;
>>>> + return 0;
>>>> +}
>>>> +
>>>> static int vma_get_mapfile(char *fname, struct vma_area *vma, DIR *mfd,
>>>> struct vma_file_info *vfi, struct vma_file_info *prev_vfi)
>>>> {
>>>> char path[32];
>>>> int flags;
>>>>
>>>> + /* Figure out if it's file mapping */
>>>> + snprintf(path, sizeof(path), "%"PRIx64"-%"PRIx64, vma->e->start, vma->e->end);
>>>> +
>>>> + if (vma_get_mapfile_flags(vma, mfd, path))
>>>> + return -1;
>>>
>>> Can we re-use the vmfd borrowing logic for fdflags too?
>>>
>>
>> You mean in the cycle below?
>> Yes, but then we have to add check for fdflags to vfi_equal, shouldn't we?
>
> Ah, indeed. This is purely about fd borrowing.
>
>>>> +
>>>> if (prev_vfi->vma && vfi_equal(vfi, prev_vfi)) {
>>>> struct vma_area *prev = prev_vfi->vma;
>>>>
>>>> @@ -209,9 +229,6 @@ static int vma_get_mapfile(char *fname, struct vma_area *vma, DIR *mfd,
>>>> return 0;
>>>> }
>>>>
>>>> - /* Figure out if it's file mapping */
>>>> - snprintf(path, sizeof(path), "%"PRIx64"-%"PRIx64, vma->e->start, vma->e->end);
>>>> -
>>>> /*
>>>> * Note that we "open" it in dumper process space
>>>> * so later we might refer to it via /proc/self/fd/vm_file_fd
>>>> diff --git a/criu/util.c b/criu/util.c
>>>> index e8ebe61..07bc16e 100644
>>>> --- a/criu/util.c
>>>> +++ b/criu/util.c
>>>> @@ -161,12 +161,13 @@ void pr_vma(unsigned int loglevel, const struct vma_area *vma_area)
>>>> return;
>>>>
>>>> vma_opt_str(vma_area, opt);
>>>> - print_on_level(loglevel, "%#"PRIx64"-%#"PRIx64" (%"PRIi64"K) prot %#x flags %#x st %#x off %#"PRIx64" "
>>>> + print_on_level(loglevel, "%#"PRIx64"-%#"PRIx64" (%"PRIi64"K) prot %#x flags %#x fdflags %#o st %#x off %#"PRIx64" "
>>>> "%s shmid: %#"PRIx64"\n",
>>>> vma_area->e->start, vma_area->e->end,
>>>> KBYTES(vma_area_len(vma_area)),
>>>> vma_area->e->prot,
>>>> vma_area->e->flags,
>>>> + vma_area->e->fdflags,
>>>> vma_area->e->status,
>>>> vma_area->e->pgoff,
>>>> opt, vma_area->e->shmid);
>>>>
>>>> _______________________________________________
>>>> CRIU mailing list
>>>> CRIU at openvz.org
>>>> https://lists.openvz.org/mailman/listinfo/criu
>>>> .
>>>>
>>>
>>
>
More information about the CRIU
mailing list