Skip to contentSkip to conversation

Check previously-unchecked buf_alloc_write() call in crypto self-test.

PATCH In review Gert Doering 2 participants 3 messages Nov 8, 2016

lore
Share & tools
lore

Gert Doering

author
#0 lore raw

PATCH 1/1 Check previously-unchecked buf_alloc_write() call in crypto self-test.

"It cannot be NULL", but since this is self-test infrastructure, assume the worst - add ASSERT() check to ensure assumptions are true.

Found by Coverity.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/crypto.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/openvpn/crypto.c +4 -1
index 6e1e46b..41dae92 100644
@@ -1028,6 +1028,7 @@ test_crypto (struct crypto_options *co, struct frame* frame)
   struct buffer encrypt_workspace = alloc_buf_gc (BUF_SIZE (frame), &gc);
   struct buffer decrypt_workspace = alloc_buf_gc (BUF_SIZE (frame), &gc);
   struct buffer buf = clear_buf();
+  void * buf_p;
 
   /* init work */
   ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));

@@ -1073,7 +1074,9 @@ test_crypto (struct crypto_options *co, struct frame* frame)
 
       /* copy source to input buf */
       buf = work;
-      memcpy (buf_write_alloc (&buf, BLEN (&src)), BPTR (&src), BLEN (&src));
+      buf_p = buf_write_alloc (&buf, BLEN (&src));
+      ASSERT(buf_p);
+      memcpy (buf_p, BPTR (&src), BLEN (&src));
 
       /* initialize work buffer with FRAME_HEADROOM bytes of prepend capacity */
       ASSERT (buf_init (&encrypt_workspace, FRAME_HEADROOM (frame)));
Signature · 1 line
2.7.3
#1 lore raw
Quoted 15 lines · Gert Doering

"It cannot be NULL", but since this is self-test infrastructure, assume the worst - add ASSERT() check to ensure assumptions are true.

Found by Coverity.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/crypto.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/openvpn/crypto.c +0 -0
index 6e1e46b..41dae92 100644

src/openvpn/crypto.c test_crypto (struct crypto_options *co, struct frame* frame) · line 1031

Quoted 4 lines
src/openvpn/crypto.c +1 -0
@@ -1028,6 +1028,7 @@ test_crypto (struct crypto_options *co, struct frame* frame)
   struct buffer encrypt_workspace = alloc_buf_gc (BUF_SIZE (frame), &gc);
   struct buffer decrypt_workspace = alloc_buf_gc (BUF_SIZE (frame), &gc);
   struct buffer buf = clear_buf();
+  void * buf_p;

Most of our code would use "void *buf_p".

src/openvpn/crypto.c test_crypto (struct crypto_options *co, struct frame* frame) · line 1082

Quoted 13 lines
src/openvpn/crypto.c +3 -1
   /* init work */
   ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));

@@ -1073,7 +1074,9 @@ test_crypto (struct crypto_options *co, struct frame* frame)
 
       /* copy source to input buf */
       buf = work;
-      memcpy (buf_write_alloc (&buf, BLEN (&src)), BPTR (&src), BLEN (&src));
+      buf_p = buf_write_alloc (&buf, BLEN (&src));
+      ASSERT(buf_p);
+      memcpy (buf_p, BPTR (&src), BLEN (&src));
 
       /* initialize work buffer with FRAME_HEADROOM bytes of prepend capacity */
       ASSERT (buf_init (&encrypt_workspace, FRAME_HEADROOM (frame)));

ACK, whether you remove the space or not.

-Steffan

author
#2 lore raw
In reply to Gert Doering

Patch has been applied to the master branch (with the offending space removed).

commit 63bdc6ce66a970f0584bbb1d4a8cae98b36ee831

Author: Gert Doering
Date:   Tue Nov 8 10:17:12 2016 +0100
     Check previously-unchecked buf_alloc_write() call in crypto self-test.

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12940.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering