[CRIU] [PATCH] restore: Use in_vma_area helper in restore_priv_vma_content

[Andrew Vagin]

On Mon, Mar 25, 2013 at 04:18:47PM +0400, Cyrill Gorcunov wrote:
> On Mon, Mar 25, 2013 at 04:09:48PM +0400, Cyrill Gorcunov wrote:
> > On Mon, Mar 25, 2013 at 04:02:28PM +0400, Andrew Vagin wrote:
> > > > @@ -301,7 +301,7 @@ static int restore_priv_vma_content(pid_t pid)
> > > >  			unsigned char buf[PAGE_SIZE];
> > > >  			void *p;
> > > >  
> > > > -			while (va >= vma->vma.end) {
> > > > +			while (!(in_vma_area(vma, va))) {
> > > 
> > > This is correct, but I don't like it. The vma list should be sorted, so
> > > the origin сondition is correct. We need to add a check, that va is in
> > > vma, right after this loop.
> > 
> > OK, letme check if I manage to come with some better patch.
> 
> Something like this one, I guess.

Acked-by: Andrew Vagin <avagin at parallels.com>

> ---
> From: Cyrill Gorcunov <gorcunov at openvz.org>
> Date: Mon, 25 Mar 2013 16:17:05 +0400
> Subject: [PATCH] restore: Validate page address in restore_priv_vma_content
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-restore.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/cr-restore.c b/cr-restore.c
> index 13273c1..f64681a 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -301,12 +301,29 @@ static int restore_priv_vma_content(pid_t pid)
>  			unsigned char buf[PAGE_SIZE];
>  			void *p;
>  
> +			/*
> +			 * The lookup os over *all* possible VMAs
> +			 * read from image file.
> +			 */
>  			while (va >= vma->vma.end) {
>  				if (vma->list.next == &rst_vmas.h)
>  					goto err_addr;
>  				vma = list_entry(vma->list.next, struct vma_area, list);
>  			}
>  
> +			/*
> +			 * Make sure the page address is inside existing VMA
> +			 * and the VMA it refers to still private one, since
> +			 * there is no guarantee that the data from pagemap is
> +			 * valid.
> +			 */
> +			if (va < vma->vma.start) {
> +				goto err_addr;
> +			} else if (unlikely(!vma_priv(&vma->vma))) {
> +				pr_err("Trying to restore page for non-private VMA\n");
> +				goto err_addr;
> +			}
> +
>  			off = (va - vma->vma.start) / PAGE_SIZE;
>  			va += PAGE_SIZE;
>  
> -- 
> 1.8.1.4
>