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);
return res;
}
[PATCH] Fix bad free in connections
3 messages · Steve Grubb, Martin Morgan
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}}
On Friday, July 21, 2017 5:03:09 AM EDT Martin Morgan wrote:
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?
Yes, I think I see the issue. vasprintf seems to not be tracked for memory allocation. So, yes I agree its correct as written. Sorry for the noise. -Steve