vma: Don't set vma_area->vm_file_fd to -1 in alloc

Submitted by Pavel Emelianov on May 19, 2016, 2:05 p.m.

Details

Message ID 573DC830.1000600@virtuozzo.com
State Rejected
Series "vma: Don't set vma_area->vm_file_fd to -1 in alloc"
Headers show

Commit Message

Pavel Emelianov May 19, 2016, 2:05 p.m.
The vma_area contains union-ed fields for dump and restore. The
generic alloc_vma_area() sets -1 to vm_file_fd which is a) dump
field and b) intersects with some restore fields. Thus at restore
time the structure that is expected to be all zeroed is not such.

Fix this and set vm_file_fd to -1 only when really needed.

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
---
 criu/proc_parse.c | 2 ++
 criu/util.c       | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/criu/proc_parse.c b/criu/proc_parse.c
index cebf21c..0133128 100644
--- a/criu/proc_parse.c
+++ b/criu/proc_parse.c
@@ -402,6 +402,7 @@  int parse_self_maps_lite(struct vm_area_list *vms)
 			return -1;
 		}
 
+		vma->vm_file_fd = -1;
 		vma->e->start = strtoul(buf, &end, 16);
 		vma->e->end = strtoul(end + 1, NULL, 16);
 		list_add_tail(&vma->list, &vms->h);
@@ -671,6 +672,7 @@  int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list)
 			goto err;
 		}
 
+		vma_area->vm_file_fd	= -1;
 		vma_area->e->start	= start;
 		vma_area->e->end	= end;
 		vma_area->e->pgoff	= pgoff;
diff --git a/criu/util.c b/criu/util.c
index e8ebe61..eab0ac2 100644
--- a/criu/util.c
+++ b/criu/util.c
@@ -800,7 +800,6 @@  struct vma_area *alloc_vma_area(void)
 	if (p) {
 		p->e = (VmaEntry *)(p + 1);
 		vma_entry__init(p->e);
-		p->vm_file_fd = -1;
 		p->e->fd = -1;
 	}
 

Comments

Cyrill Gorcunov May 19, 2016, 2:07 p.m.
On Thu, May 19, 2016 at 05:05:36PM +0300, Pavel Emelyanov wrote:
> The vma_area contains union-ed fields for dump and restore. The
> generic alloc_vma_area() sets -1 to vm_file_fd which is a) dump
> field and b) intersects with some restore fields. Thus at restore
> time the structure that is expected to be all zeroed is not such.
> 
> Fix this and set vm_file_fd to -1 only when really needed.
> 
> Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Andrey Vagin May 19, 2016, 4:26 p.m.
On Thu, May 19, 2016 at 05:05:36PM +0300, Pavel Emelyanov wrote:
> The vma_area contains union-ed fields for dump and restore. The
> generic alloc_vma_area() sets -1 to vm_file_fd which is a) dump
> field and b) intersects with some restore fields. Thus at restore
> time the structure that is expected to be all zeroed is not such.
> 
> Fix this and set vm_file_fd to -1 only when really needed.
>

I do the same and even a bit more in "[PATCH] criu: dump filemap as soon as
possible"
 
> Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
> ---
>  criu/proc_parse.c | 2 ++
>  criu/util.c       | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/criu/proc_parse.c b/criu/proc_parse.c
> index cebf21c..0133128 100644
> --- a/criu/proc_parse.c
> +++ b/criu/proc_parse.c
> @@ -402,6 +402,7 @@ int parse_self_maps_lite(struct vm_area_list *vms)
>  			return -1;
>  		}
>  
> +		vma->vm_file_fd = -1;
>  		vma->e->start = strtoul(buf, &end, 16);
>  		vma->e->end = strtoul(end + 1, NULL, 16);
>  		list_add_tail(&vma->list, &vms->h);
> @@ -671,6 +672,7 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list)
>  			goto err;
>  		}
>  
> +		vma_area->vm_file_fd	= -1;
>  		vma_area->e->start	= start;
>  		vma_area->e->end	= end;
>  		vma_area->e->pgoff	= pgoff;
> diff --git a/criu/util.c b/criu/util.c
> index e8ebe61..eab0ac2 100644
> --- a/criu/util.c
> +++ b/criu/util.c
> @@ -800,7 +800,6 @@ struct vma_area *alloc_vma_area(void)
>  	if (p) {
>  		p->e = (VmaEntry *)(p + 1);
>  		vma_entry__init(p->e);
> -		p->vm_file_fd = -1;
>  		p->e->fd = -1;
>  	}
>  
> -- 
> 2.5.0
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu