| Message ID | 1512724338-22197-1-git-send-email-steffan@karger.me |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Fix memory leak in buffer unit tests | expand |
Hi, On 08/12/17 17:12, Steffan Karger wrote: > buffer_list_push_data does not take ownership of the memory, so just feed > it stack data to plug the leak. > > Signed-off-by: Steffan Karger <steffan@karger.me> as reported by Steffan, buffer_list_push_data() takes the content of the buffer passed as argument and copies it in a new buffer. Thus the ownership of that argument still belongs to the caller which is responsible for releasing it. Using stack data just fixes the issue. Acked-by: Antonio Quartulli <a@unstable.cc> Aside from this patch, I just noted that buffer_list_push_data() suffers from the same problem as buffer_list_aggregate_separator(). The caller could pass size=0 as argument, thus leading to a 0-byte malloc. This should probably be fixed with an additional patch. Cheers, > --- > tests/unit_tests/openvpn/test_buffer.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c > index ba5aa67..c2b7f30 100644 > --- a/tests/unit_tests/openvpn/test_buffer.c > +++ b/tests/unit_tests/openvpn/test_buffer.c > @@ -77,10 +77,9 @@ static int test_buffer_list_setup(void **state) > buffer_list_push(ctx->zero_length_strings, ""); > > ctx->empty_buffers = buffer_list_new(2); > - uint8_t *data1 = malloc(1); > - uint8_t *data2 = malloc(1); > - buffer_list_push_data(ctx->empty_buffers, data1, 0); > - buffer_list_push_data(ctx->empty_buffers, data2, 0); > + uint8_t data = 0; > + buffer_list_push_data(ctx->empty_buffers, &data, 0); > + buffer_list_push_data(ctx->empty_buffers, &data, 0); > > *state = ctx; > return 0; >
Your patch has been applied to the master and release/2.4 branch.
commit 2c7c760dfbddbc9cf348bce06fa922c1217a2039 (master)
commit 888204a1e9a4278a62e9a1cb0ea1c0cdff95473e (release/2.4)
Author: Steffan Karger
Date: Fri Dec 8 10:12:18 2017 +0100
Fix memory leak in buffer unit tests
Signed-off-by: Steffan Karger <steffan@karger.me>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <1512724338-22197-1-git-send-email-steffan@karger.me>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg16055.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c index ba5aa67..c2b7f30 100644 --- a/tests/unit_tests/openvpn/test_buffer.c +++ b/tests/unit_tests/openvpn/test_buffer.c @@ -77,10 +77,9 @@ static int test_buffer_list_setup(void **state) buffer_list_push(ctx->zero_length_strings, ""); ctx->empty_buffers = buffer_list_new(2); - uint8_t *data1 = malloc(1); - uint8_t *data2 = malloc(1); - buffer_list_push_data(ctx->empty_buffers, data1, 0); - buffer_list_push_data(ctx->empty_buffers, data2, 0); + uint8_t data = 0; + buffer_list_push_data(ctx->empty_buffers, &data, 0); + buffer_list_push_data(ctx->empty_buffers, &data, 0); *state = ctx; return 0;
buffer_list_push_data does not take ownership of the memory, so just feed it stack data to plug the leak. Signed-off-by: Steffan Karger <steffan@karger.me> --- tests/unit_tests/openvpn/test_buffer.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)