Re: gnutls_memset: use explicit_bzero

On 27.03.2019 08:24, Nikos Mavrogiannopoulos wrote:
> On Mon, 2019-03-11 at 00:02 +0100, Maciej S. Szmigiero wrote:
>>> That is, use the glibc function when available and the second
>>> parameter is zero.
>>>
>>> Resolves #230
>>>
>>> Signed-off-by: Nikos Mavrogiannopoulos <[email protected]>
>>> ---(..)
>>> --- a/lib/safe-memfuncs.c
>>> +++ b/lib/safe-memfuncs.c
>>> @@ -33,14 +30,18 @@
>>>   * This function will operate similarly to memset(), but will
>>>   * not be optimized out by the compiler.
>>>   *
>>> - * Returns: void.
>>> - *
>>>   * Since: 3.4.0
>>>   **/
>>>  void gnutls_memset(void *data, int c, size_t size)
>>>  {
>>> -	volatile unsigned volatile_zero = 0;
>>> +	volatile unsigned volatile_zero;
>>>  	volatile char *vdata = (volatile char*)data;
>>> +#ifdef HAVE_EXPLICIT_BZERO
>>> +	if (c == 0) {
>>> +		explicit_bzero(data, size);
>>
>> Shouldn't the function return here?
>>
>> Because otherwise it is doing the zeroing twice:
>> first time via explicit_bzero(),
>> second time via a volatile trick below.
> 
> You are right. Would you like to send a merge request fixing that?

While I don't have a gitlab account to open a merge request there
I have attached a patch made by git-format-patch.
Hope this way will work, too.

> regards,
> Nikos

Thanks,
Maciej

_______________________________________________
Gnutls-help mailing list
[email protected]
http://lists.gnupg.org/mailman/listinfo/gnutls-help

From 6fa8a6f77f7a0cc2656405923687e4d4ed137cf3 Mon Sep 17 00:00:00 2001
From: "Maciej S. Szmigiero" <[email protected]>
Date: Thu, 28 Mar 2019 23:04:13 +0100
Subject: [PATCH] gnutls_memset(): calling explicit_bzero() is enough to
 zero-fill a buffer

If we use explicit_bzero() to zero-fill a buffer in gnutls_memset() we
don't need to zero it again via a volatile trick later in this function.

Signed-off-by: Maciej S. Szmigiero <[email protected]>
---
 lib/safe-memfuncs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/safe-memfuncs.c b/lib/safe-memfuncs.c
index a9db39d2a7e2..b957b432cd87 100644
--- a/lib/safe-memfuncs.c
+++ b/lib/safe-memfuncs.c
@@ -39,6 +39,7 @@ void gnutls_memset(void *data, int c, size_t size)
 #ifdef HAVE_EXPLICIT_BZERO
 	if (c == 0) {
 		explicit_bzero(data, size);
+		return;
 	}
 #endif
 	volatile_zero = 0;