| Message ID | 20210402173414.14216-1-gert@greenie.muc.de |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel] Fix potential NULL ptr crash if compiled with DMALLOC | expand |
Hi, On 02/04/2021 19:34, Gert Doering wrote: > In the unlikely case that we are compiled with -DDMALLOC *and* > malloc() returns NULL, there is an uncaught memset() which would > crash then. Remove the memset(), as the right the next operation > after check_malloc_return() is a mempcy() which will overwrite > the whole memory block anyway. > > Trac: #586 > > Signed-off-by: Gert Doering <gert@greenie.muc.de> > --- > src/openvpn/buffer.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c > index 48bf25d5..e7031a4f 100644 > --- a/src/openvpn/buffer.c > +++ b/src/openvpn/buffer.c > @@ -706,7 +706,6 @@ string_alloc(const char *str, struct gc_arena *gc) > */ > #ifdef DMALLOC > ret = openvpn_dmalloc(file, line, n); > - memset(ret, 0, n); > #else > ret = calloc(1, n); > #endif > As discussed with Gert on IRC, we could rather add a "if (ret)" before doing memset. However the DMALLOC code path is barely tested at all and we don't want to introduce more changes than needed. So for now, we can live with this change. Acked-by: Antonio Quartulli <antonio@openvpn.net>
Patch has been applied to the master, release/2.5 and release/2.4 branch
(crazily important bugfix!).
commit e2acfad40c0d79ce7fd431c380d7466d383bcefa (master)
commit acf52dda9f4cb117e9d020dd06fccd7ecb90d303 (release/2.5)
commit a7263a125199c6d11710ecf50f9a07424369fdbc (release/2.4)
Author: Gert Doering
Date: Fri Apr 2 19:34:14 2021 +0200
Fix potential NULL ptr crash if compiled with DMALLOC
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@openvpn.net>
Message-Id: <20210402173414.14216-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg21981.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c index 48bf25d5..e7031a4f 100644 --- a/src/openvpn/buffer.c +++ b/src/openvpn/buffer.c @@ -706,7 +706,6 @@ string_alloc(const char *str, struct gc_arena *gc) */ #ifdef DMALLOC ret = openvpn_dmalloc(file, line, n); - memset(ret, 0, n); #else ret = calloc(1, n); #endif
In the unlikely case that we are compiled with -DDMALLOC *and* malloc() returns NULL, there is an uncaught memset() which would crash then. Remove the memset(), as the right the next operation after check_malloc_return() is a mempcy() which will overwrite the whole memory block anyway. Trac: #586 Signed-off-by: Gert Doering <gert@greenie.muc.de> --- src/openvpn/buffer.c | 1 - 1 file changed, 1 deletion(-)