[CRIU] [PATCH] proc_parse: fix vma file open mode recognition
Stanislav Kinsburskiy
skinsbursky at virtuozzo.com
Tue Jul 5 05:24:44 PDT 2016
05.07.2016 14:19, Pavel Emelyanov пишет:
> 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?
>
Here is the code:
- 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;
1) If file was opened with O_WRONLY, this mode is simply lost.
2) If file was opened with O_RDWR, but mapping is private, resulting mode will be O_RDONLY
>>>>> 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