[CRIU] Re: [PATCH] fdset: Make fdset open helpers to work in all-or-nothing manner

[Pavel Emelyanov]

On 01/31/2012 01:38 PM, Cyrill Gorcunov wrote:
> When cr_fdset_open or prep_cr_fdset_for_restore is called
> the caller expects either all operations are successfull
> or there is some error happened and NULL returned, but
> instead at moment partially filled fdset might be returned.
> 
> Fix it -- we either should get all or nothing.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  crtools.c |   18 ++++++++++++------
>  1 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/crtools.c b/crtools.c
> index 926d26b..d7f956d 100644
> --- a/crtools.c
> +++ b/crtools.c
> @@ -157,15 +157,18 @@ struct cr_fdset *cr_fdset_open(int pid, unsigned long use_mask, struct cr_fdset
>  			pr_perror("Unable to open %s\n", path);
>  			goto err;
>  		}
> +		cr_fdset->fds[i] = ret;
>  
>  		pr_debug("Opened %s with %d\n", path, ret);
>  		if (write_img(ret, &fdset_template[i].magic))
>  			goto err;
> -
> -		cr_fdset->fds[i] = ret;
>  	}
> -err:
> +
>  	return cr_fdset;
> +
> +err:
> +	close_cr_fdset(&cr_fdset);
> +	return NULL;

NAK.

Code now works like this


fdset = cr_fdset_open(NULL);

        fdset2 = cr_fdset_open(fdset);

cr_fdset_close(&fdset);

If you free the fdset in cr_fdset_open 2nd call you get the memory corruption.

>  }
>  
>  struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
> @@ -194,6 +197,7 @@ struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
>  			pr_perror("Unable to open %s\n", path);
>  			goto err;
>  		}
> +		cr_fdset->fds[i] = ret;
>  
>  		pr_debug("Opened %s with %d\n", path, ret);
>  		if (read_img(ret, &magic) < 0)
> @@ -203,11 +207,13 @@ struct cr_fdset *prep_cr_fdset_for_restore(int pid, unsigned long use_mask)
>  			pr_err("Magic doesn't match for %s\n", path);
>  			goto err;
>  		}
> -
> -		cr_fdset->fds[i] = ret;
>  	}
> -err:
> +
>  	return cr_fdset;
> +
> +err:
> +	close_cr_fdset(&cr_fdset);
> +	return NULL;
>  }
>  
>  void close_cr_fdset(struct cr_fdset **cr_fdset)