I give this one a feature-ACK as it seems to fix the buffer.c line 313 issue:
<http://thread.gmane.org/gmane.network.openvpn.devel/5346> All the best, -- Samuli Seppänen Community Manager OpenVPN Technologies, Inc irc freenode net: mattock > In commit bee92b479414d12035b0422f81ac5fcfe14fa645 the gc_malloc() was > hardened > to always require a gc_arena object for garbage collection. Some places in > the > code expected the old behaviour of a normal malloc() in these cases, that is a > memory allocation without garbage collection. > > This old behaviour is partly restored by allowing string_alloc() to do a > non-gc > based allocation if no gc_arena object is available. In addition some other > places string_alloc() will now be called with a gc_arena pointer where such an > object is available. > > The alloc_buf() function has also been refactored to not use gc_malloc() at > all. > > v2: - removes a memleak when --ifconfig-ipv6 is used several times > - makes string_alloc() behave properly if DMALLOC is enabled > > Signed-off-by: David Sommerseth <dav...@redhat.com> > --- > buffer.c | 32 ++++++++++++++++++++++++++++---- > init.c | 2 +- > openvpn.c | 2 +- > options.c | 7 ++++++- > pf.c | 2 +- > ssl_verify.c | 2 ++ > 6 files changed, 39 insertions(+), 8 deletions(-) > > diff --git a/buffer.c b/buffer.c > index c39bbcb..fca6a90 100644 > --- a/buffer.c > +++ b/buffer.c > @@ -54,11 +54,21 @@ alloc_buf_debug (size_t size, const char *file, int line) > alloc_buf (size_t size) > #endif > { > + struct buffer buf; > + > + if (!buf_size_valid (size)) > + buf_size_error (size); > + buf.capacity = (int)size; > + buf.offset = 0; > + buf.len = 0; > #ifdef DMALLOC > - return alloc_buf_gc_debug (size, NULL, file, line); > + buf.data = openvpn_dmalloc (file, line, size); > #else > - return alloc_buf_gc (size, NULL); > + buf.data = calloc (1, size); > #endif > + check_malloc_return(buf.data); > + > + return buf; > } > > struct buffer > @@ -515,11 +525,25 @@ string_alloc (const char *str, struct gc_arena *gc) > const int n = strlen (str) + 1; > char *ret; > > + if (gc) { > +#ifdef DMALLOC > + ret = (char *) gc_malloc_debug (n, false, gc, file, line); > +#else > + ret = (char *) gc_malloc (n, false, gc); > +#endif > + } else { > + /* If there are no garbage collector available, it's expected > + * that the caller cleans up afterwards. This is coherent with the > + * earlier behaviour when gc_malloc() would be called with gc == NULL > + */ > #ifdef DMALLOC > - ret = (char *) gc_malloc_debug (n, false, gc, file, line); > + ret = openvpn_dmalloc (file, line, n); > + memset(ret, 0, n); > #else > - ret = (char *) gc_malloc (n, false, gc); > + ret = calloc(1, n); > #endif > + check_malloc_return(ret); > + } > memcpy (ret, str, n); > return ret; > } > diff --git a/init.c b/init.c > index 525f441..f0c3693 100644 > --- a/init.c > +++ b/init.c > @@ -3012,7 +3012,7 @@ do_close_ifconfig_pool_persist (struct context *c) > static void > do_inherit_env (struct context *c, const struct env_set *src) > { > - c->c2.es = env_set_create (NULL); > + c->c2.es = env_set_create (&c->c2.gc); > c->c2.es_owned = true; > env_set_inherit (c->c2.es, src); > } > diff --git a/openvpn.c b/openvpn.c > index f5f2bce..84289d2 100644 > --- a/openvpn.c > +++ b/openvpn.c > @@ -164,7 +164,7 @@ main (int argc, char *argv[]) > gc_init (&c.gc); > > /* initialize environmental variable store */ > - c.es = env_set_create (NULL); > + c.es = env_set_create (&c.gc); > #ifdef WIN32 > set_win_sys_path_via_env (c.es); > #endif > diff --git a/options.c b/options.c > index 6b8ae22..a0b3431 100644 > --- a/options.c > +++ b/options.c > @@ -4291,7 +4291,7 @@ add_option (struct options *options, > { > unsigned int netbits; > char * ipv6_local; > - > + > VERIFY_PERMISSION (OPT_P_UP); > if ( get_ipv6_addr( p[1], NULL, &netbits, &ipv6_local, msglevel ) && > ipv6_addr_safe( p[2] ) ) > @@ -4301,6 +4301,11 @@ add_option (struct options *options, > msg( msglevel, "ifconfig-ipv6: /netbits must be between 64 and > 124, not '/%d'", netbits ); > goto err; > } > + > + if (options->ifconfig_ipv6_local) > + /* explicitly ignoring this is a const char */ > + free ((char *) options->ifconfig_ipv6_local); > + > options->ifconfig_ipv6_local = ipv6_local; > options->ifconfig_ipv6_netbits = netbits; > options->ifconfig_ipv6_remote = p[2]; > diff --git a/pf.c b/pf.c > index 6b4cba4..79915fa 100644 > --- a/pf.c > +++ b/pf.c > @@ -566,7 +566,7 @@ pf_init_context (struct context *c) > if (plugin_call (c->plugins, OPENVPN_PLUGIN_ENABLE_PF, NULL, NULL, > c->c2.es) == OPENVPN_PLUGIN_FUNC_SUCCESS) > { > event_timeout_init (&c->c2.pf.reload, 1, now); > - c->c2.pf.filename = string_alloc (pf_file, NULL); > + c->c2.pf.filename = string_alloc (pf_file, &c->c2.gc); > c->c2.pf.enabled = true; > #ifdef ENABLE_DEBUG > if (check_debug_level (D_PF_DEBUG)) > diff --git a/ssl_verify.c b/ssl_verify.c > index e45f149..37d4982 100644 > --- a/ssl_verify.c > +++ b/ssl_verify.c > @@ -83,6 +83,7 @@ set_common_name (struct tls_session *session, const char > *common_name) > } > if (common_name) > { > + /* FIXME: Last alloc will never be freed */ > session->common_name = string_alloc (common_name, NULL); > #ifdef ENABLE_PF > { > @@ -703,6 +704,7 @@ man_def_auth_set_client_reason (struct tls_multi *multi, > const char *client_reas > multi->client_reason = NULL; > } > if (client_reason && strlen (client_reason)) > + /* FIXME: Last alloc will never be freed */ > multi->client_reason = string_alloc (client_reason, NULL); > } >
