replace direct calls to memset_s() with commonly used macros

samba provides macros for zeroing various structures in memory,
and all code uses them instead of relying on memset_s().
However, a few places use memset_s() directly.  Replace these
usages with macros for consistency and to be able to replace
memset_s() easier.

A few notes.

Commit 03a50d8f7d872b6ef701d12 "lib:util: Check memset_s() error
code in talloc_keep_secret_destructor()" (Aug-2022) added a check
for error return from memset_s().  This is the only place in whole
codebase which bothers about doing this.  But I've difficult time
figuring out the intention.  Was there a real case when this code
path is actually executed?

Commit 7658c9bf0a9c99e3f200571 "lib:crypto: Remove redundant array
zeroing" (Nov-2023) removed the OTHER line from the two lines used
to zero memory in here.  Initially the code used both memset_s()
*and* ZERO_ARRAY_LEN(), the former has been removed.  This change
removes the other - memset_s(), reintroducing ZERO_ARRAY_LEN().
Here however, it's probably better to use BURN_PTR instead of
ZERO_ARRAY - in this place and a few lines above.

Commit 8dddea2ceda40f2365bd6b1 "lib:talloc: Use memset_s() to avoid
the call gets optimized out" (Feb-2024) is a recent commit which
introduces memset_s().  However, it does not seem like it makes
any difference whatsoever for a testsuite, or that it actually
needs to clean up the memory to begin with.

We've quite an assortment of all this memory zeroing stuff.  Also
it is repeated in replace.h and memory.h (two sets in these files
are different but has big intersection).  I'd say, to fix this mess,
things from replace.h should be removed in favour of memory.h, and
necessary includes added, but this is for the next time.  We also
have lots of direct usages of memset_s() in heimdal code.

Cc: Joseph Sutton <josephsutton@catalyst.net.nz>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Pavel Filipenský <pfilipensky@samba.org>

diff --git a/lib/cmdline/cmdline.c b/lib/cmdline/cmdline.c
index a42707238f62733286a1605c32ee9a74ec8de718..161ba8874bf34fbaa53a2394172c41b124588a90 100644 (file)
--- a/lib/cmdline/cmdline.c
+++ b/lib/cmdline/cmdline.c
@@ -358,7 +358,7 @@ bool samba_cmdline_burn(int argc, char *argv[])
                                p += ulen;
                        }
 
-                       memset_s(p, strlen(p), '\0', strlen(p));
+                       BURN_PTR_SIZE(p, strlen(p));
                        burnt = true;
                }
        }

diff --git a/lib/crypto/gnutls_sp800_108.c b/lib/crypto/gnutls_sp800_108.c
index fb0aa03921329499b8a69d9b45fbeb9b4afec698..a1af0cf7c60eb662fabcfb7c37602c0b38c6d82f 100644 (file)
--- a/lib/crypto/gnutls_sp800_108.c
+++ b/lib/crypto/gnutls_sp800_108.c
@@ -223,7 +223,7 @@ out:
        }
        if (!NT_STATUS_IS_OK(status)) {
                /* Hide the evidence. */
-               memset_s(KO, KO_len, 0, KO_idx);
+               ZERO_ARRAY_LEN(KO, KO_idx);
        }
 
        return status;

diff --git a/lib/talloc/testsuite.c b/lib/talloc/testsuite.c
index dc0039940fff913dac88a62856a23eee066f351a..f9b98cf9cffe9c2eee601a873cd6fea045e8586a 100644 (file)
--- a/lib/talloc/testsuite.c
+++ b/lib/talloc/testsuite.c
@@ -2149,7 +2149,7 @@ static bool test_magic_protection(void)
                 *
                 * Real attacks would attempt to set a real destructor.
                 */
-               memset_s(p1, 32, '\0', 32);
+               BURN_PTR_SIZE(p1, 32);
 
                /* Then the attack takes effect when the memory's freed. */
                talloc_free(pool);

diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c
index 84e814f2ae86f405336b285e3c0374def465a713..9372612d8a3e4e12bb1f2fd06aa3300a057949e7 100644 (file)
--- a/lib/util/data_blob.c
+++ b/lib/util/data_blob.c
@@ -95,7 +95,7 @@ clear a DATA_BLOB's contents
 _PUBLIC_ void data_blob_clear(DATA_BLOB *d)
 {
        if (d->data) {
-               memset_s(d->data, d->length, 0, d->length);
+               ZERO_ARRAY_LEN(d->data, d->length);
        }
 }
 

diff --git a/lib/util/talloc_keep_secret.c b/lib/util/talloc_keep_secret.c
index 21048659e5df70a0d39f61669b10af4cb3646d53..eb5bb80ff37fc330a965909526ea7c3c5e61ee69 100644 (file)
--- a/lib/util/talloc_keep_secret.c
+++ b/lib/util/talloc_keep_secret.c
@@ -22,26 +22,13 @@
 
 static int talloc_keep_secret_destructor(void *ptr)
 {
-       int ret;
        size_t size = talloc_get_size(ptr);
 
        if (unlikely(size == 0)) {
                return 0;
        }
 
-       ret = memset_s(ptr, size, 0, size);
-       if (unlikely(ret != 0)) {
-               char *msg = NULL;
-               int ret2;
-               ret2 = asprintf(&msg,
-                               "talloc_keep_secret_destructor: memset_s() failed: %s",
-                               strerror(ret));
-               if (ret2 != -1) {
-                       smb_panic(msg);
-               } else {
-                       smb_panic("talloc_keep_secret_destructor: memset_s() failed");
-               }
-       }
+       BURN_PTR_SIZE(ptr, size);
 
        return 0;
 }

diff --git a/librpc/ndr/util.c b/librpc/ndr/util.c
index 0eb7eba9e5ed57a236b027bce2be1cda72060706..472f44ace565772f2888dcd0f9a68c0214b94bd1 100644 (file)
--- a/librpc/ndr/util.c
+++ b/librpc/ndr/util.c
@@ -32,5 +32,5 @@ _PUBLIC_ void ndr_print_sockaddr_storage(struct ndr_print *ndr, const char *name
 
 _PUBLIC_ void ndr_zero_memory(void *ptr, size_t len)
 {
-       memset_s(ptr, len, 0, len);
+       ZERO_ARRAY_LEN(ptr, len);
 }