[CRIU] [PATCH] proc_parse: fix vma file open mode recognition
Pavel Emelyanov
xemul at virtuozzo.com
Tue Jul 5 04:18:18 PDT 2016
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.
>> >
>> > 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