vz7: files -- Don't access value from stack in outer contex

Submitted by Cyrill Gorcunov on July 12, 2017, 2:41 p.m.

Details

Message ID 1499870498-14222-1-git-send-email-gorcunov@virtuozzo.com
State Rejected
Series "vz7: files -- Don't access value from stack in outer contex"
Headers show

Commit Message

Cyrill Gorcunov July 12, 2017, 2:41 p.m.
The struct fd_link link allocated in inner context might be
freed before dump call (depending on compiler), instead
use a safer approach.

Signed-off-by: Cyrill Gorcunov <gorcunov@virtuozzo.com>
---
 criu/files.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/files.c b/criu/files.c
index 91dfcef..420a83f 100644
--- a/criu/files.c
+++ b/criu/files.c
@@ -410,8 +410,11 @@  static const struct fdtype_ops *get_mem_dev_ops(struct fd_parms *p, int minor)
 
 static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
 {
+	struct fd_link *link_old = p->link;
 	int maj = major(p->stat.st_rdev);
 	const struct fdtype_ops *ops;
+	struct fd_link link;
+	int err;
 
 	switch (maj) {
 	case MEM_MAJOR:
@@ -426,8 +429,6 @@  static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
 		char more[32];
 
 		if (is_tty(p->stat.st_rdev, p->stat.st_dev)) {
-			struct fd_link link;
-
 			if (fill_fdlink(lfd, p, &link))
 				return -1;
 			p->link = &link;
@@ -436,11 +437,15 @@  static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
 		}
 
 		sprintf(more, "%d:%d", maj, minor(p->stat.st_rdev));
-		return dump_unsupp_fd(p, lfd, img, "chr", more);
+		err = dump_unsupp_fd(p, lfd, img, "chr", more);
+		p->link = link_old;
+		return err;
 	}
 	}
 
-	return do_dump_gen_file(p, lfd, ops, img);
+	err = do_dump_gen_file(p, lfd, ops, img);
+	p->link = link_old;
+	return err;
 }
 
 static int check_blkdev(struct fd_parms *p, int lfd)

Comments

Andrey Vagin July 12, 2017, 8:57 p.m.
Do we need it for the upstream criu? Could you send a patch for it?

On Wed, Jul 12, 2017 at 05:41:38PM +0300, Cyrill Gorcunov wrote:
> The struct fd_link link allocated in inner context might be
> freed before dump call (depending on compiler), instead
> use a safer approach.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@virtuozzo.com>
> ---
>  criu/files.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/criu/files.c b/criu/files.c
> index 91dfcef..420a83f 100644
> --- a/criu/files.c
> +++ b/criu/files.c
> @@ -410,8 +410,11 @@ static const struct fdtype_ops *get_mem_dev_ops(struct fd_parms *p, int minor)
>  
>  static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
>  {
> +	struct fd_link *link_old = p->link;
>  	int maj = major(p->stat.st_rdev);
>  	const struct fdtype_ops *ops;
> +	struct fd_link link;
> +	int err;
>  
>  	switch (maj) {
>  	case MEM_MAJOR:
> @@ -426,8 +429,6 @@ static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
>  		char more[32];
>  
>  		if (is_tty(p->stat.st_rdev, p->stat.st_dev)) {
> -			struct fd_link link;
> -
>  			if (fill_fdlink(lfd, p, &link))
>  				return -1;
>  			p->link = &link;
> @@ -436,11 +437,15 @@ static int dump_chrdev(struct fd_parms *p, int lfd, struct cr_img *img)
>  		}
>  
>  		sprintf(more, "%d:%d", maj, minor(p->stat.st_rdev));
> -		return dump_unsupp_fd(p, lfd, img, "chr", more);
> +		err = dump_unsupp_fd(p, lfd, img, "chr", more);
> +		p->link = link_old;
> +		return err;
>  	}
>  	}
>  
> -	return do_dump_gen_file(p, lfd, ops, img);
> +	err = do_dump_gen_file(p, lfd, ops, img);
> +	p->link = link_old;
> +	return err;
>  }
>  
>  static int check_blkdev(struct fd_parms *p, int lfd)
> -- 
> 2.7.5
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu