diff options
author | Arne Schwabe | 2022-12-27 15:02:45 +0100 |
---|---|---|
committer | Gert Doering | 2022-12-27 20:07:42 +0100 |
commit | 09267c1ccde6a8f0671c785413ca847e97bbc0a5 (patch) | |
tree | 199df230aeaf05eb6089d6e10838dbd0fa0076f4 | |
parent | f5919a42e4a6772c4116d19cd09132bedb6d77f5 (diff) | |
download | openvpn-09267c1ccde6a8f0671c785413ca847e97bbc0a5.zip openvpn-09267c1ccde6a8f0671c785413ca847e97bbc0a5.tar.gz |
Replace realloc with new gc_realloc function
The realloc logic has the problem that it relies on the memory being
deallocated by uninit_options rather than by freeing the gc. This
does not always happen in all code path. Especially the crypto selftest
run by make check will not call uninit_options.
This introduces a gc_realloc function that ensures that the pointer is
instead freed when gc_free is called.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20221227140249.3524943-2-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25829.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
(cherry picked from commit e7f2169772f90f9bf158a17f5656a6a985e74e31)
-rw-r--r-- | src/openvpn/buffer.c | 33 | ||||
-rw-r--r-- | src/openvpn/buffer.h | 13 | ||||
-rw-r--r-- | src/openvpn/options.c | 6 | ||||
-rw-r--r-- | tests/unit_tests/openvpn/test_buffer.c | 32 |
4 files changed, 80 insertions, 4 deletions
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 575d45a..e3d5ba2 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -412,6 +412,39 @@ gc_malloc(size_t size, bool clear, struct gc_arena *a) return ret; } +void * +gc_realloc(void *ptr, size_t size, struct gc_arena *a) +{ + void *ret = realloc(ptr, size); + check_malloc_return(ret); + if (a) + { + if (ptr && ptr != ret) + { + /* find the old entry and modify it if realloc changed + * the pointer */ + struct gc_entry_special *e = NULL; + for (e = a->list_special; e != NULL; e = e->next) + { + if (e->addr == ptr) + { + break; + } + } + ASSERT(e); + ASSERT(e->addr == ptr); + e->addr = ret; + } + else if (!ptr) + { + /* sets e->addr to newptr */ + gc_addspecial(ret, free, a); + } + } + + return ret; +} + void x_gc_free(struct gc_arena *a) { diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h index fece633..185226f 100644 --- a/src/openvpn/buffer.h +++ b/src/openvpn/buffer.h @@ -187,6 +187,19 @@ struct buffer string_alloc_buf(const char *str, struct gc_arena *gc); void gc_addspecial(void *addr, void (*free_function)(void *), struct gc_arena *a); +/** + * allows to realloc a pointer previously allocated by gc_malloc or gc_realloc + * + * @note only use this function on pointers returned by gc_malloc or re_alloc + * with the same gc_arena + * + * @param ptr Pointer of the previously allocated memory + * @param size New size + * @param a gc_arena to use + * @return new pointer + */ +void * +gc_realloc(void *ptr, size_t size, struct gc_arena *a); #ifdef BUF_INIT_TRACKING #define buf_init(buf, offset) buf_init_debug(buf, offset, __FILE__, __LINE__) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 4e018fb..e454b2a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -918,12 +918,10 @@ uninit_options(struct options *o) { if (o->connection_list) { - free(o->connection_list->array); CLEAR(*o->connection_list); } if (o->remote_list) { - free(o->remote_list->array); CLEAR(*o->remote_list); } if (o->gc_owned) @@ -2173,7 +2171,7 @@ alloc_connection_entry(struct options *options, const int msglevel) if (l->len == l->capacity) { int capacity = l->capacity + CONNECTION_LIST_SIZE; - struct connection_entry **ce = realloc(l->array, capacity*sizeof(struct connection_entry *)); + struct connection_entry **ce = gc_realloc(l->array, capacity*sizeof(struct connection_entry *), &options->gc); if (ce == NULL) { msg(msglevel, "Unable to process more connection options: out of memory. Number of entries = %d", l->len); @@ -2206,7 +2204,7 @@ alloc_remote_entry(struct options *options, const int msglevel) if (l->len == l->capacity) { int capacity = l->capacity + CONNECTION_LIST_SIZE; - struct remote_entry **re = realloc(l->array, capacity*sizeof(struct remote_entry *)); + struct remote_entry **re = gc_realloc(l->array, capacity*sizeof(struct remote_entry *), &options->gc); if (re == NULL) { msg(msglevel, "Unable to process more remote options: out of memory. Number of entries = %d", l->len); diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c index ac70166..9e3b1d2 100644 --- a/tests/unit_tests/openvpn/test_buffer.c +++ b/tests/unit_tests/openvpn/test_buffer.c @@ -231,6 +231,37 @@ test_buffer_free_gc_two(void **state) gc_free(&gc); } + +static void +test_buffer_gc_realloc(void **state) +{ + struct gc_arena gc = gc_new(); + + void *p1 = gc_realloc(NULL, 512, &gc); + void *p2 = gc_realloc(NULL, 512, &gc); + + assert_ptr_not_equal(p1, p2); + + memset(p1, '1', 512); + memset(p2, '2', 512); + + p1 = gc_realloc(p1, 512, &gc); + + /* allocate 512kB to ensure the pointer needs to change */ + void *p1new = gc_realloc(p1, 512ul * 1024, &gc); + assert_ptr_not_equal(p1, p1new); + + void *p2new = gc_realloc(p2, 512ul * 1024, &gc); + assert_ptr_not_equal(p2, p2new); + + void *p3 = gc_realloc(NULL, 512, &gc); + memset(p3, '3', 512); + + + gc_free(&gc); +} + + int main(void) { @@ -259,6 +290,7 @@ main(void) test_buffer_list_teardown), cmocka_unit_test(test_buffer_free_gc_one), cmocka_unit_test(test_buffer_free_gc_two), + cmocka_unit_test(test_buffer_gc_realloc), }; return cmocka_run_group_tests_name("buffer", tests, NULL, NULL); |