[CRIU] [PATCH] net: Relax xmalloc-ing (and fix NULL deref)

[Andrei Vagin]

Applied, thanks!

On Fri, Jun 16, 2017 at 06:19:04PM +0300, Pavel Emelyanov wrote:
> There's potential NULL-derefernece in dump_netns_con() -- two xmalloc
> results are not checked. However, since there's a huge set of these
> xmallocs, I propose to relax the whole thing with one big xmalloc and
> xptr_pull() helper.
> 
> Signed-off-by: Pavel Emelyanov <xemul at virtuozzo.com>
> ---
>  criu/net.c | 55 +++++++++++++++++++++----------------------------------
>  1 file changed, 21 insertions(+), 34 deletions(-)
> 
> diff --git a/criu/net.c b/criu/net.c
> index 6849cd2..90aa947 100644
> --- a/criu/net.c
> +++ b/criu/net.c
> @@ -1564,6 +1564,7 @@ static inline int dump_iptables(struct cr_imgset *fds)
>  
>  static int dump_netns_conf(struct ns_id *ns, struct cr_imgset *fds)
>  {
> +	void *buf, *o_buf;
>  	int ret = -1;
>  	int i;
>  	NetnsEntry netns = NETNS_ENTRY__INIT;
> @@ -1580,8 +1581,16 @@ static int dump_netns_conf(struct ns_id *ns, struct cr_imgset *fds)
>  	list_for_each_entry(p, &ns->net.ids, node)
>  		i++;
>  
> -	netns.nsids = xmalloc(sizeof(NetnsId *) * i);
> -	ids = xmalloc(sizeof(NetnsId) * i);
> +	o_buf = buf = xmalloc(
> +			i * (sizeof(NetnsId*) + sizeof(NetnsId)) +
> +			size4 * (sizeof(SysctlEntry*) + sizeof(SysctlEntry)) * 2 +
> +			size6 * (sizeof(SysctlEntry*) + sizeof(SysctlEntry)) * 2
> +		     );
> +	if (!buf)
> +		goto out;
> +
> +	netns.nsids = xptr_pull_s(&buf, i * sizeof(NetnsId*));
> +	ids = xptr_pull_s(&buf, i * sizeof(NetnsId));
>  	i = 0;
>  	list_for_each_entry(p, &ns->net.ids, node) {
>  		netns_id__init(&ids[i]);
> @@ -1594,18 +1603,10 @@ static int dump_netns_conf(struct ns_id *ns, struct cr_imgset *fds)
>  
>  	netns.n_def_conf4 = size4;
>  	netns.n_all_conf4 = size4;
> -	netns.def_conf4 = xmalloc(sizeof(SysctlEntry *) * size4);
> -	if (!netns.def_conf4)
> -		goto err_free;
> -	netns.all_conf4 = xmalloc(sizeof(SysctlEntry *) * size4);
> -	if (!netns.all_conf4)
> -		goto err_free;
> -	def_confs4 = xmalloc(sizeof(SysctlEntry) * size4);
> -	if (!def_confs4)
> -		goto err_free;
> -	all_confs4 = xmalloc(sizeof(SysctlEntry) * size4);
> -	if (!all_confs4)
> -		goto err_free;
> +	netns.def_conf4 = xptr_pull_s(&buf, size4 * sizeof(SysctlEntry*));
> +	netns.all_conf4 = xptr_pull_s(&buf, size4 * sizeof(SysctlEntry*));
> +	def_confs4 = xptr_pull_s(&buf, size4 * sizeof(SysctlEntry));
> +	all_confs4 = xptr_pull_s(&buf, size4 * sizeof(SysctlEntry));
>  
>  	for (i = 0; i < size4; i++) {
>  		sysctl_entry__init(&def_confs4[i]);
> @@ -1618,18 +1619,10 @@ static int dump_netns_conf(struct ns_id *ns, struct cr_imgset *fds)
>  
>  	netns.n_def_conf6 = size6;
>  	netns.n_all_conf6 = size6;
> -	netns.def_conf6 = xmalloc(sizeof(SysctlEntry *) * size6);
> -	if (!netns.def_conf6)
> -		goto err_free;
> -	netns.all_conf6 = xmalloc(sizeof(SysctlEntry *) * size6);
> -	if (!netns.all_conf6)
> -		goto err_free;
> -	def_confs6 = xmalloc(sizeof(SysctlEntry) * size6);
> -	if (!def_confs6)
> -		goto err_free;
> -	all_confs6 = xmalloc(sizeof(SysctlEntry) * size6);
> -	if (!all_confs6)
> -		goto err_free;
> +	netns.def_conf6 = xptr_pull_s(&buf, size6 * sizeof(SysctlEntry*));
> +	netns.all_conf6 = xptr_pull_s(&buf, size6 * sizeof(SysctlEntry*));
> +	def_confs6 = xptr_pull_s(&buf, size6 * sizeof(SysctlEntry));
> +	all_confs6 = xptr_pull_s(&buf, size6 * sizeof(SysctlEntry));
>  
>  	for (i = 0; i < size6; i++) {
>  		sysctl_entry__init(&def_confs6[i]);
> @@ -1663,14 +1656,8 @@ static int dump_netns_conf(struct ns_id *ns, struct cr_imgset *fds)
>  
>  	ret = pb_write_one(img_from_set(fds, CR_FD_NETNS), &netns, PB_NETNS);
>  err_free:
> -	xfree(netns.def_conf4);
> -	xfree(netns.all_conf4);
> -	xfree(def_confs4);
> -	xfree(all_confs4);
> -	xfree(netns.def_conf6);
> -	xfree(netns.all_conf6);
> -	xfree(def_confs6);
> -	xfree(all_confs6);
> +	xfree(o_buf);
> +out:
>  	return ret;
>  }
>  
> -- 
> 2.1.4
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu