diff mbox series

[Openvpn-devel] Fix potential NULL ptr crash if compiled with DMALLOC

Message ID 20210402173414.14216-1-gert@greenie.muc.de
State Accepted
Headers
Series [Openvpn-devel] Fix potential NULL ptr crash if compiled with DMALLOC |

Commit Message

Gert Doering April 2, 2021, 6:34 a.m. UTC 
  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(-)

Comments

Antonio Quartulli April 3, 2021, 10:39 p.m. UTC | #1 
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>
Gert Doering April 3, 2021, 10:59 p.m. UTC | #2 
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 mbox series

Patch

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