[CRIU] [PATCH] mem: fix typo in determining an address of parent vma

[Andrew Vagin]

On Tue, May 13, 2014 at 09:54:58AM -0700, Filipe Brandenburger wrote:
> It turns out that when you apply this patch then it will actually COW
> too much...
> 
> For instance, the sep_tcs test case of cow01 fails for me with this patch in.

Do you see two other patches? On my host cow01 passes with these patches.

> 
> The reason is that the vmas on parent and child have same start and
> end but are not really COWed from one another.
> 
> The problem is that the zero pages on the child not returned by
> pr.get_pagemap() are not handled by restore_priv_vma_content() so they
> end up with the contents of the parent instead of zeroed out contents.
> (There seems to be a similar problem with the file private mmap in
> file_tcs as well but I didn't go far enough in troubleshooting that
> one yet.)

Do you look at the code below the "/* Remove pages, which were not shared
with a child */" comment?

> 
> I'd suggest holding on this patch until those other issues are
> addressed, otherwise you'll go from having not COWed pages with the
> right memory contents to having pages with the wrong contents in them
> which is worse.
> 
> I think the patches to fix traversal of COW vmas belong before this
> one on the order of patches.
> 
> Cheers,
> Filipe
> 
> 
> On Tue, May 13, 2014 at 4:39 AM, Andrey Vagin <avagin at openvz.org> wrote:
> > Look at this hunk from 7659c995f58f:
> > -    paddr = decode_pointer(vma_premmaped_start(&p->vma));
> > +    paddr = decode_pointer(vma->premmaped_addr);
> >
> > Obviously we want to use p->premmaped_addr instead of
> > vma->premmaped_addr.
> >
> > Fixes: 7659c995f58f ("vm: don't overwrite vma->shmid for private mappings")
> > Reported-by: Filipe Brandenburger <filbranden at google.com>
> > Signed-off-by: Andrey Vagin <avagin at openvz.org>
> > ---
> >  cr-restore.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/cr-restore.c b/cr-restore.c
> > index d79da59..897c3fc 100644
> > --- a/cr-restore.c
> > +++ b/cr-restore.c
> > @@ -260,7 +260,7 @@ static int map_private_vma(pid_t pid, struct vma_area *vma, void **tgt_addr,
> >
> >                 pr_info("COW 0x%016"PRIx64"-0x%016"PRIx64" 0x%016"PRIx64" vma\n",
> >                         vma->e->start, vma->e->end, vma->e->pgoff);
> > -               paddr = decode_pointer(vma->premmaped_addr);
> > +               paddr = decode_pointer(p->premmaped_addr);
> >         }
> >
> >         *pvma = p;
> > --
> > 1.8.5.3
> >