From martin.morgan at roswellpark.org  Fri Jul 21 11:03:09 2017
From: martin.morgan at roswellpark.org (Martin Morgan)
Date: Fri, 21 Jul 2017 05:03:09 -0400
Subject: [Rd] [PATCH] Fix bad free in connections
In-Reply-To: <2125974.DtE9TkYUl0@x2>
References: <2125974.DtE9TkYUl0@x2>
Message-ID: <25a319a3-d822-671f-06e9-5e2034f56c16@roswellpark.org>

On 07/20/2017 05:04 PM, Steve Grubb wrote:
> Hello,
> 
> There are times when b points to buf which is a stack variable. This
> leads to a bad free. The current test actually guarantees the stack
> will try to get freed. Simplest to just drop the variable and directly
> test if b should get freed.
> 
> 
> Signed-off-by: Steve Grubb <sgrubb at redhat.com>
> 
> 
> Index: src/main/connections.c
> ===================================================================
> --- src/main/connections.c	(revision 72935)
> +++ src/main/connections.c	(working copy)
> @@ -421,7 +421,6 @@
>       char buf[BUFSIZE], *b = buf;
>       int res;
>       const void *vmax = NULL; /* -Wall*/
> -    int usedVasprintf = FALSE;
>       va_list aq;
>   
>       va_copy(aq, ap);
> @@ -434,7 +433,7 @@
>   	    b = buf;
>   	    buf[BUFSIZE-1] = '\0';
>   	    warning(_("printing of extremely long output is truncated"));
> -	} else usedVasprintf = TRUE;
> +	}
>       }
>   #else
>       if(res >= BUFSIZE) { /* res is the desired output length */
> @@ -481,7 +480,7 @@
>       } else
>   	con->write(b, 1, res, con);
>       if(vmax) vmaxset(vmax);
> -    if(usedVasprintf) free(b);
> +    if(b != buf) free(b);

The code can be exercised with

   z = paste(rep("a", 11000), collapse="")
   f = fifo("foo", "w+")
   writeLines(z, f)

If the macro HAVE_VASPRINTF is not defined, then b is the result of 
R_alloc(), and it is not appropriate to free(b).

If the macro is defined we go through

	res = vasprintf(&b, format, ap);
	if (res < 0) {
	    b = buf;
	    buf[BUFSIZE-1] = '\0';
	    warning(_("printing of extremely long output is truncated"));
	} else usedVasprintf = TRUE;

b gets reallocated when

     res = vasprintf(&b, format, ap);

is successful and res >= 0. usedVasprintf is then set to TRUE, and 
free(b) called.

It seems like the code is correct as written?

Martin Morgan (the real other Martin M*)

>       return res;
>   }
> 
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
> 


This email message may contain legally privileged and/or...{{dropped:2}}



[Rd] [PATCH] Fix bad free in connections
Martin Morgan