Re: [PATCH] elf: Use memset_zero to erase data

["H.J. Lu" <hjl.tools@gmail.com>]

On Wed, Feb 9, 2022 at 8:11 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> ---
>  elf/cache.c             |  8 ++++----
>  elf/circleload1.c       |  2 +-
>  elf/dl-load.c           |  2 +-
>  elf/dl-map-segments.h   |  2 +-
>  elf/dl-minimal-malloc.c |  2 +-
>  elf/dl-profile.c        |  8 ++++----
>  elf/dl-reloc.c          |  5 +++--
>  elf/dl-sort-maps.c      |  4 ++--
>  elf/dl-tls.c            | 30 ++++++++++++++++--------------
>  elf/neededtest.c        |  2 +-
>  elf/neededtest2.c       |  2 +-
>  elf/neededtest3.c       |  2 +-
>  elf/neededtest4.c       |  2 +-
>  elf/sprof.c             |  2 +-
>  14 files changed, 38 insertions(+), 35 deletions(-)
>
> diff --git a/elf/cache.c b/elf/cache.c
> index dbf4c83a7a..8987fad828 100644
> --- a/elf/cache.c
> +++ b/elf/cache.c
> @@ -608,7 +608,7 @@ save_cache (const char *cache_name)
>        file_entries = xmalloc (file_entries_size);
>
>        /* Fill in the header.  */
> -      memset (file_entries, '\0', sizeof (struct cache_file));
> +      memset_zero (file_entries, sizeof (struct cache_file));
>        memcpy (file_entries->magic, CACHEMAGIC, sizeof CACHEMAGIC - 1);
>
>        file_entries->nlibs = cache_entry_old_count;
> @@ -625,7 +625,7 @@ save_cache (const char *cache_name)
>        file_entries_new = xmalloc (file_entries_new_size);
>
>        /* Fill in the header.  */
> -      memset (file_entries_new, '\0', sizeof (struct cache_file_new));
> +      memset_zero (file_entries_new, sizeof (struct cache_file_new));
>        memcpy (file_entries_new->magic, CACHEMAGIC_NEW,
>               sizeof CACHEMAGIC_NEW - 1);
>        memcpy (file_entries_new->version, CACHE_VERSION,
> @@ -738,7 +738,7 @@ save_cache (const char *cache_name)
>        if (opt_format != opt_format_new)
>         {
>           char zero[pad];
> -         memset (zero, '\0', pad);
> +         memset_zero (zero, pad);
>           if (write (fd, zero, pad) != (ssize_t) pad)
>             error (EXIT_FAILURE, errno, _("Writing of cache data failed"));
>         }
> @@ -1088,7 +1088,7 @@ save_aux_cache (const char *aux_cache_name)
>      = xmalloc (file_entries_size + total_strlen);
>
>    /* Fill in the header of the auxiliary cache.  */
> -  memset (file_entries, '\0', sizeof (struct aux_cache_file));
> +  memset_zero (file_entries, sizeof (struct aux_cache_file));
>    memcpy (file_entries->magic, AUX_CACHEMAGIC, sizeof AUX_CACHEMAGIC - 1);
>
>    file_entries->nlibs = cache_entry_count;
> diff --git a/elf/circleload1.c b/elf/circleload1.c
> index 990ff84a84..e9981395d8 100644
> --- a/elf/circleload1.c
> +++ b/elf/circleload1.c
> @@ -21,7 +21,7 @@ check_loaded_objects (const char **loaded)
>    if (n)
>      {
>        found = (int *) alloca (sizeof (int) * n);
> -      memset (found, 0, sizeof (int) * n);
> +      memset_zero (found, sizeof (int) * n);
>      }
>
>    printf("   Name\n");
> diff --git a/elf/dl-load.c b/elf/dl-load.c
> index 5b0ff41ee1..87fd476c52 100644
> --- a/elf/dl-load.c
> +++ b/elf/dl-load.c
> @@ -961,7 +961,7 @@ _dl_map_object_from_fd (const char *name, const char *origname, int fd,
>    if (mode & __RTLD_OPENEXEC)
>      {
>        assert (nsid == LM_ID_BASE);
> -      memset (&id, 0, sizeof (id));
> +      memset_zero (&id, sizeof (id));
>      }
>    else
>      {
> diff --git a/elf/dl-map-segments.h b/elf/dl-map-segments.h
> index 172692b120..6d4759973f 100644
> --- a/elf/dl-map-segments.h
> +++ b/elf/dl-map-segments.h
> @@ -171,7 +171,7 @@ _dl_map_segments (struct link_map *l, int fd,
>                                    GLRO(dl_pagesize), c->prot|PROT_WRITE) < 0)
>                      return DL_MAP_SEGMENTS_ERROR_MPROTECT;
>                  }
> -              memset ((void *) zero, '\0', zeropage - zero);
> +              memset_zero ((void *) zero, zeropage - zero);
>                if (__glibc_unlikely ((c->prot & PROT_WRITE) == 0))
>                  __mprotect ((caddr_t) (zero & ~(GLRO(dl_pagesize) - 1)),
>                              GLRO(dl_pagesize), c->prot);
> diff --git a/elf/dl-minimal-malloc.c b/elf/dl-minimal-malloc.c
> index 7cca54208d..024e640dcd 100644
> --- a/elf/dl-minimal-malloc.c
> +++ b/elf/dl-minimal-malloc.c
> @@ -98,7 +98,7 @@ __minimal_free (void *ptr)
>      {
>        /* Since this is rare, we clear the freed block here
>          so that calloc can presume malloc returns cleared memory.  */
> -      memset (alloc_last_block, '\0', alloc_ptr - alloc_last_block);
> +      memset_zero (alloc_last_block, alloc_ptr - alloc_last_block);
>        alloc_ptr = alloc_last_block;
>      }
>  }
> diff --git a/elf/dl-profile.c b/elf/dl-profile.c
> index 9359be7c33..65957a6ad5 100644
> --- a/elf/dl-profile.c
> +++ b/elf/dl-profile.c
> @@ -274,7 +274,7 @@ _dl_start_profile (void)
>
>    memcpy (&gmon_hdr.cookie[0], GMON_MAGIC, sizeof (gmon_hdr.cookie));
>    gmon_hdr.version = GMON_SHOBJ_VERSION;
> -  memset (gmon_hdr.spare, '\0', sizeof (gmon_hdr.spare));
> +  memset_zero (gmon_hdr.spare, sizeof (gmon_hdr.spare));
>
>    /* Create the hist_hdr we expect or write.  */
>    struct real_gmon_hist_hdr
> @@ -308,8 +308,8 @@ _dl_start_profile (void)
>    if (sizeof (hist_hdr.dimen) >= sizeof ("seconds"))
>      {
>        memcpy (hist_hdr.dimen, "seconds", sizeof ("seconds"));
> -      memset (hist_hdr.dimen + sizeof ("seconds"), '\0',
> -             sizeof (hist_hdr.dimen) - sizeof ("seconds"));
> +      memset_zero (hist_hdr.dimen + sizeof ("seconds"),
> +                  sizeof (hist_hdr.dimen) - sizeof ("seconds"));
>      }
>    else
>      strncpy (hist_hdr.dimen, "seconds", sizeof (hist_hdr.dimen));
> @@ -355,7 +355,7 @@ _dl_start_profile (void)
>        /* We have to create the file.  */
>        char buf[GLRO(dl_pagesize)];
>
> -      memset (buf, '\0', GLRO(dl_pagesize));
> +      memset_zero (buf, GLRO(dl_pagesize));
>
>        if (__lseek (fd, expected_size & ~(GLRO(dl_pagesize) - 1), SEEK_SET) == -1)
>         {
> diff --git a/elf/dl-reloc.c b/elf/dl-reloc.c
> index 771a34bd14..3b4bee421f 100644
> --- a/elf/dl-reloc.c
> +++ b/elf/dl-reloc.c
> @@ -157,8 +157,9 @@ _dl_nothread_init_static_tls (struct link_map *map)
>  #endif
>
>    /* Initialize the memory.  */
> -  memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
> -         '\0', map->l_tls_blocksize - map->l_tls_initimage_size);
> +  memset_zero (__mempcpy (dest, map->l_tls_initimage,
> +                         map->l_tls_initimage_size),
> +              map->l_tls_blocksize - map->l_tls_initimage_size);
>  }
>  #endif /* !PTHREAD_IN_LIBC */
>
> diff --git a/elf/dl-sort-maps.c b/elf/dl-sort-maps.c
> index 9e9d53ec47..2cb38cec9a 100644
> --- a/elf/dl-sort-maps.c
> +++ b/elf/dl-sort-maps.c
> @@ -40,7 +40,7 @@ _dl_sort_maps_original (struct link_map **maps, unsigned int nmaps,
>
>    unsigned int i = 0;
>    uint16_t seen[nmaps];
> -  memset (seen, 0, nmaps * sizeof (seen[0]));
> +  memset_zero (seen, nmaps * sizeof (seen[0]));
>    while (1)
>      {
>        /* Keep track of which object we looked at this round.  */
> @@ -115,7 +115,7 @@ _dl_sort_maps_original (struct link_map **maps, unsigned int nmaps,
>        if (++i == nmaps)
>         break;
>      next_clear:
> -      memset (&seen[i], 0, (nmaps - i) * sizeof (seen[0]));
> +      memset_zero (&seen[i], (nmaps - i) * sizeof (seen[0]));
>
>      next:;
>      }
> diff --git a/elf/dl-tls.c b/elf/dl-tls.c
> index 093cdddb7e..d3e1e70475 100644
> --- a/elf/dl-tls.c
> +++ b/elf/dl-tls.c
> @@ -447,7 +447,7 @@ _dl_allocate_tls_storage (void)
>
>    /* Clear the TCB data structure.  We can't ask the caller (i.e.
>       libpthread) to do it, because we will initialize the DTV et al.  */
> -  memset (result, '\0', TLS_TCB_SIZE);
> +  memset_zero (result, TLS_TCB_SIZE);
>  #elif TLS_DTV_AT_TP
>    /* Pre-TCB and TCB come before the TLS blocks.  The layout computed
>       in _dl_determine_tlsoffset assumes that the TCB is aligned to the
> @@ -461,7 +461,8 @@ _dl_allocate_tls_storage (void)
>    /* Clear the TCB data structure and TLS_PRE_TCB_SIZE bytes before
>       it.  We can't ask the caller (i.e. libpthread) to do it, because
>       we will initialize the DTV et al.  */
> -  memset (result - TLS_PRE_TCB_SIZE, '\0', TLS_PRE_TCB_SIZE + TLS_TCB_SIZE);
> +  memset_zero (result - TLS_PRE_TCB_SIZE,
> +              TLS_PRE_TCB_SIZE + TLS_TCB_SIZE);
>  #endif
>
>    /* Record the value of the original pointer for later
> @@ -511,8 +512,8 @@ _dl_resize_dtv (dtv_t *dtv, size_t max_modid)
>    newp[0].counter = newsize;
>
>    /* Clear the newly allocated part.  */
> -  memset (newp + 2 + oldsize, '\0',
> -         (newsize - oldsize) * sizeof (dtv_t));
> +  memset_zero (newp + 2 + oldsize,
> +              (newsize - oldsize) * sizeof (dtv_t));
>
>    /* Return the generation counter.  */
>    return &newp[1];
> @@ -605,9 +606,9 @@ _dl_allocate_tls_init (void *result, bool init_tls)
>              behaviour.   */
>           if (map->l_ns != LM_ID_BASE && !init_tls)
>             continue;
> -         memset (__mempcpy (dest, map->l_tls_initimage,
> -                            map->l_tls_initimage_size), '\0',
> -                 map->l_tls_blocksize - map->l_tls_initimage_size);
> +         memset_zero (__mempcpy (dest, map->l_tls_initimage,
> +                                 map->l_tls_initimage_size),
> +                      map->l_tls_blocksize - map->l_tls_initimage_size);
>         }
>
>        total += cnt;
> @@ -712,9 +713,9 @@ allocate_and_init (struct link_map *map)
>      oom ();
>
>    /* Initialize the memory.  */
> -  memset (__mempcpy (result.val, map->l_tls_initimage,
> -                    map->l_tls_initimage_size),
> -         '\0', map->l_tls_blocksize - map->l_tls_initimage_size);
> +  memset_zero (__mempcpy (result.val, map->l_tls_initimage,
> +                         map->l_tls_initimage_size),
> +              map->l_tls_blocksize - map->l_tls_initimage_size);
>
>    return result;
>  }
> @@ -1053,8 +1054,8 @@ cannot create TLS data structures"));
>
>        listp->len = TLS_SLOTINFO_SURPLUS;
>        listp->next = NULL;
> -      memset (listp->slotinfo, '\0',
> -             TLS_SLOTINFO_SURPLUS * sizeof (struct dtv_slotinfo));
> +      memset_zero (listp->slotinfo,
> +                  TLS_SLOTINFO_SURPLUS * sizeof (struct dtv_slotinfo));
>        /* Synchronize with _dl_update_slotinfo.  */
>        atomic_store_release (&prevp->next, listp);
>      }
> @@ -1082,8 +1083,9 @@ init_one_static_tls (struct pthread *curp, struct link_map *map)
>  # endif
>
>    /* Initialize the memory.  */
> -  memset (__mempcpy (dest, map->l_tls_initimage, map->l_tls_initimage_size),
> -         '\0', map->l_tls_blocksize - map->l_tls_initimage_size);
> +  memset_zero (__mempcpy (dest, map->l_tls_initimage,
> +                         map->l_tls_initimage_size),
> +              map->l_tls_blocksize - map->l_tls_initimage_size);
>  }
>
>  void
> diff --git a/elf/neededtest.c b/elf/neededtest.c
> index 3cea499314..1ab8380123 100644
> --- a/elf/neededtest.c
> +++ b/elf/neededtest.c
> @@ -21,7 +21,7 @@ check_loaded_objects (const char **loaded)
>    if (n)
>      {
>        found = (int *) alloca (sizeof (int) * n);
> -      memset (found, 0, sizeof (int) * n);
> +      memset_zero (found, sizeof (int) * n);
>      }
>
>    printf("   Name\n");
> diff --git a/elf/neededtest2.c b/elf/neededtest2.c
> index 17c75f2ba3..eb67692828 100644
> --- a/elf/neededtest2.c
> +++ b/elf/neededtest2.c
> @@ -21,7 +21,7 @@ check_loaded_objects (const char **loaded)
>    if (n)
>      {
>        found = (int *) alloca (sizeof (int) * n);
> -      memset (found, 0, sizeof (int) * n);
> +      memset_zero (found, sizeof (int) * n);
>      }
>
>    printf("   Name\n");
> diff --git a/elf/neededtest3.c b/elf/neededtest3.c
> index 41970cf2c7..5cc7a4453e 100644
> --- a/elf/neededtest3.c
> +++ b/elf/neededtest3.c
> @@ -21,7 +21,7 @@ check_loaded_objects (const char **loaded)
>    if (n)
>      {
>        found = (int *) alloca (sizeof (int) * n);
> -      memset (found, 0, sizeof (int) * n);
> +      memset_zero (found, sizeof (int) * n);
>      }
>
>    printf("   Name\n");
> diff --git a/elf/neededtest4.c b/elf/neededtest4.c
> index 0ae0b7ff47..8e88194dcf 100644
> --- a/elf/neededtest4.c
> +++ b/elf/neededtest4.c
> @@ -21,7 +21,7 @@ check_loaded_objects (const char **loaded)
>    if (n)
>      {
>        found = (int *) alloca (sizeof (int) * n);
> -      memset (found, 0, sizeof (int) * n);
> +      memset_zero (found, sizeof (int) * n);
>      }
>
>    printf("   Name\n");
> diff --git a/elf/sprof.c b/elf/sprof.c
> index 405fbcbf38..b950e61a16 100644
> --- a/elf/sprof.c
> +++ b/elf/sprof.c
> @@ -860,7 +860,7 @@ load_profdata (const char *name, struct shobj *shobj)
>
>    memcpy (&gmon_hdr.cookie[0], GMON_MAGIC, sizeof (gmon_hdr.cookie));
>    gmon_hdr.version = GMON_SHOBJ_VERSION;
> -  memset (gmon_hdr.spare, '\0', sizeof (gmon_hdr.spare));
> +  memset_zero (gmon_hdr.spare, sizeof (gmon_hdr.spare));
>
>    /* Create the hist_hdr we expect or write.  */
>    struct real_gmon_hist_hdr hist_hdr;
> --
> 2.34.1
>

Sent by accident.  Please ignore this.

-- 
H.J.