Use bzero(3) instead of its pattern

It was blessed by POSIX.1-2001, and GCC says that it won't go away,
possibly ever.

memset(3) is dangerous, as the 2nd and 3rd arguments can be accidentally
swapped --who remembers what's the order of the 2nd and 3rd parameters
to memset(3) without checking the manual page or some code that uses
it?--.  Some recent compilers may be able to catch that via some
warnings, but those are not infalible.  And even if compiler warnings
could always catch that, the time lost in fixing or checking the docs is
lost for no clear gain.  Having a sane API that is unambiguous is the
Right Thing (tm); and that API is bzero(3).

If someone doesn't believe memset(3) is error-prone, please read the
book "Unix Network Programming", Volume 1, 3rd Edition by Stevens, et
al., Section 1.2.  See a stackoverflow reference in the link below[1].

bzero(3) had a bad fame in the bad old days, because some ancient
systems (I'm talking of many decades ago) shipped a broken version of
bzero(3).  We can assume that all systems in which current shadow utils
can be built, have a working version of bzero(3) --if not, please fix
your broken system; don't blame the programmer--.

One reason that some use today to avoid bzero(3) in favor of memset(3)
is that memset(3) is more often used; but that's a circular reasoning.
Even if bzero(3) wasn't supported by the system, it would need to be
invented.  It's the right API.

Another reason that some argue is that POSIX.1-2008 removed the
specification of bzero(3).  That's not a problem, because GCC will
probably support it forever, and even if it didn't, we can redefine it
like we do with memzero().  bzero(3) is just a one-liner wrapper around
memset(3).

Link: [1] <https://stackoverflow.com/a/17097978>
Cc: Christian Göttsche <cgzones@googlemail.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Iker Pedrosa <ipedrosa@redhat.com>
Signed-off-by: Alejandro Colomar <alx@kernel.org>

diff --git a/lib/idmapping.c b/lib/idmapping.c
index d9f9cd5230d2f8a65435e08c5efd2b0e188c93d1..a2e74380bf1f016932dbcd670e96007a3d56f458 100644 (file)
--- a/lib/idmapping.c
+++ b/lib/idmapping.c
@@ -11,6 +11,7 @@
 #include <limits.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <strings.h>
 
 #include "alloc.h"
 #include "prototypes.h"
@@ -176,7 +177,7 @@ void write_mapping(int proc_dir_fd, int ranges, const struct map_range *mappings
        }
 
        /* Lockdown new{g,u}idmap by dropping all unneeded capabilities. */
-       memset(data, 0, sizeof(data));
+       bzero(data, sizeof(data));
        data[0].effective = CAP_TO_MASK(cap);
        /*
         * When uid 0 from the ancestor userns is supposed to be mapped into

diff --git a/lib/memzero.h b/lib/memzero.h
index 99e2beca5c6ad28c12ad5dbb7e658cc6d1877cc6..1137e830d3236bfb7b4435bce92e43861b0c1d8b 100644 (file)
--- a/lib/memzero.h
+++ b/lib/memzero.h
@@ -33,7 +33,7 @@ memzero(void *ptr, size_t size)
 #elif defined(HAVE_EXPLICIT_BZERO)
        explicit_bzero(ptr, size);
 #else
-       memset(ptr, '\0', size);
+       bzero(ptr, size);
        __asm__ __volatile__ ("" : : "r"(ptr) : "memory");
 #endif
 }

diff --git a/lib/pam_pass_non_interactive.c b/lib/pam_pass_non_interactive.c
index cd4d78e00286945bfb428a271b963494a05cd26a..aec02ad8bbcdaedbddbdb47f7b75e7eecc1f2f14 100644 (file)
--- a/lib/pam_pass_non_interactive.c
+++ b/lib/pam_pass_non_interactive.c
@@ -13,6 +13,8 @@
 #include <string.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <strings.h>
+
 #include <security/pam_appl.h>
 
 #include "alloc.h"
@@ -94,8 +96,8 @@ static int ni_conv (int num_msg,
 failed_conversation:
        for (count=0; count < num_msg; count++) {
                if (NULL != responses[count].resp) {
-                       memset (responses[count].resp, 0,
-                               strlen (responses[count].resp));
+                       bzero(responses[count].resp,
+                             strlen(responses[count].resp));
                        free (responses[count].resp);
                        responses[count].resp = NULL;
                }

diff --git a/lib/salt.c b/lib/salt.c
index dc242ffa8b6fb4278757cac26cc378df5976f8e6..529d59cace4d087ad51568ab1bfa68d893a33811 100644 (file)
--- a/lib/salt.c
+++ b/lib/salt.c
@@ -20,6 +20,8 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <strings.h>
+
 #include "prototypes.h"
 #include "defines.h"
 #include "getdef.h"
@@ -319,7 +321,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size)
 {
        static char salt[MAX_SALT_SIZE + 6];
 
-       memset (salt, '\0', MAX_SALT_SIZE + 6);
+       bzero(salt, MAX_SALT_SIZE + 6);
 
        assert (salt_size >= MIN_SALT_SIZE &&
                salt_size <= MAX_SALT_SIZE);
@@ -358,7 +360,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size)
        const char *method;
        unsigned long rounds = 0;
 
-       memset (result, '\0', GENSALT_SETTING_SIZE);
+       bzero(result, GENSALT_SETTING_SIZE);
 
        if (NULL != meth)
                method = meth;
@@ -406,7 +408,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size)
                         method);
                salt_len = MAX_SALT_SIZE;
                rounds = 0;
-               memset (result, '\0', GENSALT_SETTING_SIZE);
+               bzero(result, GENSALT_SETTING_SIZE);
        }
 
 #if USE_XCRYPT_GENSALT
@@ -418,7 +420,7 @@ static /*@observer@*/const char *gensalt (size_t salt_size)
                /* Avoid -Wunused-but-set-variable. */
                salt_len = GENSALT_SETTING_SIZE - 1;
                rounds = 0;
-               memset (result, '.', salt_len);
+               memset(result, '.', salt_len);
                result[salt_len] = '\0';
        }
 

diff --git a/src/groupmod.c b/src/groupmod.c
index 7fd02d6f5a7083fd346c75627bedf5bf642388bb..11aca7f0c1a540a3d405e66c32abcb4fe6d9a6e4 100644 (file)
--- a/src/groupmod.c
+++ b/src/groupmod.c
@@ -17,6 +17,7 @@
 #include <grp.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <strings.h>
 #include <sys/types.h>
 #ifdef ACCT_TOOLS_SETUID
 #ifdef USE_PAM
@@ -229,7 +230,7 @@ static void grp_update (void)
                         * shadowed password, we force the creation of a
                         * gshadow entry when a new password is requested.
                         */
-                       memset (&sgrp, 0, sizeof sgrp);
+                       bzero(&sgrp, sizeof sgrp);
                        sgrp.sg_name   = xstrdup (grp.gr_name);
                        sgrp.sg_passwd = xstrdup (grp.gr_passwd);
                        sgrp.sg_adm    = &empty;

diff --git a/src/grpconv.c b/src/grpconv.c
index 57d8d58ec498f52b9e918378aa1de5245853c5d4..34ed7ad3851bd316de781575e326746f63e1176c 100644 (file)
--- a/src/grpconv.c
+++ b/src/grpconv.c
@@ -21,9 +21,11 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <strings.h>
 #include <time.h>
 #include <unistd.h>
 #include <getopt.h>
+
 #include "nscd.h"
 #include "sssd.h"
 #include "prototypes.h"
@@ -198,7 +200,7 @@ int main (int argc, char **argv)
                        static char *empty = 0;
 
                        /* add new shadow group entry */
-                       memset (&sgent, 0, sizeof sgent);
+                       bzero(&sgent, sizeof sgent);
                        sgent.sg_name = gr->gr_name;
                        sgent.sg_passwd = gr->gr_passwd;
                        sgent.sg_adm = &empty;

diff --git a/src/pwconv.c b/src/pwconv.c
index 356802c4ce0f765a208ed3544a29e61d063ff354..4f388d256a9cb052aacbfdba15ef37b6da027e0d 100644 (file)
--- a/src/pwconv.c
+++ b/src/pwconv.c
@@ -40,9 +40,11 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <strings.h>
 #include <time.h>
 #include <unistd.h>
 #include <getopt.h>
+
 #include "defines.h"
 #include "getdef.h"
 #include "prototypes.h"
@@ -237,7 +239,7 @@ int main (int argc, char **argv)
                        spent = *sp;
                } else {
                        /* add new shadow entry */
-                       memset (&spent, 0, sizeof spent);
+                       bzero(&spent, sizeof spent);
                        spent.sp_namp   = pw->pw_name;
                        spent.sp_min    = getdef_num ("PASS_MIN_DAYS", -1);
                        spent.sp_max    = getdef_num ("PASS_MAX_DAYS", -1);

diff --git a/src/usermod.c b/src/usermod.c
index 72a98f1f8ea2c3499d1e628cbbe7700f18131dfe..63e1eb9b5f021b45353baf381ba5207bf6ec0908 100644 (file)
--- a/src/usermod.c
+++ b/src/usermod.c
@@ -27,6 +27,7 @@
 #endif                         /* USE_PAM */
 #endif                         /* ACCT_TOOLS_SETUID */
 #include <stdio.h>
+#include <strings.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <time.h>
@@ -1737,7 +1738,7 @@ static void usr_update (void)
                         *    a shadowed password
                         *  + aging information is requested
                         */
-                       memset (&spent, 0, sizeof spent);
+                       bzero(&spent, sizeof spent);
                        spent.sp_namp   = user_name;
 
                        /* The user explicitly asked for a shadow feature.