[Libguestfs] [libguestfs-common PATCH 05/12] options: replace NULL-termination with number-of-elements in get_keys()

Richard W.M. Jones rjones at redhat.com
Tue Jun 28 14:24:54 UTC 2022


On Tue, Jun 28, 2022 at 01:49:08PM +0200, Laszlo Ersek wrote:
> Currently, get_keys() returns a NULL-terminated array of (char*) elements.
> In a later patch in this series, we'll want to change the element type to
> a structure type, at which point the NULL-termination would become
> awkward. Replace the NULL-termination scheme with the number-of-elements
> scheme. No behavioral changes.
> 
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1809453
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---
>  options/options.h |  4 +++-
>  options/decrypt.c | 17 +++++++-------
>  options/keys.c    | 24 ++++++++++++++------
>  3 files changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/options/options.h b/options/options.h
> index 80df91a85f98..d7a3aeff6f41 100644
> --- a/options/options.h
> +++ b/options/options.h
> @@ -149,11 +149,13 @@ extern void inspect_mount_root (guestfs_h *g, const char *root);
>  #define inspect_mount() inspect_mount_handle (g, ks)
>  extern void print_inspect_prompt (void);
>  
>  /* in key.c */
>  extern char *read_key (const char *param);
> -extern char **get_keys (struct key_store *ks, const char *device, const char *uuid);
> +extern char **get_keys (struct key_store *ks, const char *device, const char *uuid,
> +                        size_t *nr_matches);
> +extern void free_keys (char **keys, size_t nr_matches);
>  extern struct key_store *key_store_add_from_selector (struct key_store *ks, const char *selector);
>  extern struct key_store *key_store_import_key (struct key_store *ks, const struct key_store_key *key);
>  extern void free_key_store (struct key_store *ks);
>  
>  /* in options.c */
> diff --git a/options/decrypt.c b/options/decrypt.c
> index 1cd7b627e264..c25b5888d8b4 100644
> --- a/options/decrypt.c
> +++ b/options/decrypt.c
> @@ -122,14 +122,14 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
>    const char *mountable;
>  
>    while ((mountable = *mnt_scan++) != NULL) {
>      CLEANUP_FREE char *type = NULL;
>      CLEANUP_FREE char *uuid = NULL;
> -    CLEANUP_FREE_STRING_LIST char **keys = NULL;
> +    char **keys;
> +    size_t nr_matches;
>      CLEANUP_FREE char *mapname = NULL;
> -    const char * const *key_scan;
> -    const char *key;
> +    size_t scan;
>  
>      type = guestfs_vfs_type (g, mountable);
>      if (type == NULL)
>        continue;
>  
> @@ -142,37 +142,38 @@ decrypt_mountables (guestfs_h *g, const char * const *mountables,
>        continue;
>  
>      /* Grab the keys that we should try with this device, based on device name,
>       * or UUID (if any).
>       */
> -    keys = get_keys (ks, mountable, uuid);
> -    assert (keys[0] != NULL);
> +    keys = get_keys (ks, mountable, uuid, &nr_matches);
> +    assert (nr_matches > 0);
>  
>      /* Generate a node name for the plaintext (decrypted) device node. */
>      if (uuid == NULL || asprintf (&mapname, "luks-%s", uuid) == -1)
>        mapname = make_mapname (mountable);
>  
>      /* Try each key in turn. */
> -    key_scan = (const char * const *)keys;
> -    while ((key = *key_scan++) != NULL) {
> +    for (scan = 0; scan < nr_matches; ++scan) {
> +      const char *key = keys[scan];
>        int r;
>  
>        guestfs_push_error_handler (g, NULL, NULL);
>        r = guestfs_cryptsetup_open (g, mountable, key, mapname, -1);
>        guestfs_pop_error_handler (g);
>  
>        if (r == 0)
>          break;
>      }
>  
> -    if (key == NULL)
> +    if (scan == nr_matches)
>        error (EXIT_FAILURE, 0,
>               _("could not find key to open LUKS encrypted %s.\n\n"
>                 "Try using --key on the command line.\n\n"
>                 "Original error: %s (%d)"),
>               mountable, guestfs_last_error (g), guestfs_last_errno (g));
>  
> +    free_keys (keys, nr_matches);
>      decrypted_some = true;
>    }
>  
>    return decrypted_some;
>  }
> diff --git a/options/keys.c b/options/keys.c
> index 8713372a305e..1d97d980a460 100644
> --- a/options/keys.c
> +++ b/options/keys.c
> @@ -124,11 +124,12 @@ read_first_line_from_file (const char *filename)
>  /* Return the key(s) matching this particular device from the
>   * keystore.  There may be multiple.  If none are read from the
>   * keystore, ask the user.
>   */
>  char **
> -get_keys (struct key_store *ks, const char *device, const char *uuid)
> +get_keys (struct key_store *ks, const char *device, const char *uuid,
> +          size_t *nr_matches)
>  {
>    size_t i, j, nmemb;
>    char **r;
>    char *s;
>  
> @@ -137,18 +138,16 @@ get_keys (struct key_store *ks, const char *device, const char *uuid)
>     */
>    nmemb = 1;
>    if (ks && ks->nr_keys > nmemb)
>      nmemb = ks->nr_keys;
>  
> -  /* make room for the terminating NULL */
> -  if (nmemb == (size_t)-1)
> +  if (nmemb > (size_t)-1 / sizeof *r)
>      error (EXIT_FAILURE, 0, _("size_t overflow"));
> -  nmemb++;
>  
> -  r = calloc (nmemb, sizeof (char *));
> +  r = malloc (nmemb * sizeof *r);
>    if (r == NULL)
> -    error (EXIT_FAILURE, errno, "calloc");
> +    error (EXIT_FAILURE, errno, "malloc");
>  
>    j = 0;
>  
>    if (ks) {
>      for (i = 0; i < ks->nr_keys; ++i) {
> @@ -175,16 +174,27 @@ get_keys (struct key_store *ks, const char *device, const char *uuid)
>    if (j == 0) {
>      /* Key not found in the key store, ask the user for it. */
>      s = read_key (device);
>      if (!s)
>        error (EXIT_FAILURE, 0, _("could not read key from user"));
> -    r[0] = s;
> +    r[j++] = s;
>    }
>  
> +  *nr_matches = j;
>    return r;
>  }
>  
> +void
> +free_keys (char **keys, size_t nr_matches)
> +{
> +  size_t i;
> +
> +  for (i = 0; i < nr_matches; ++i)
> +    free (keys[i]);
> +  free (keys);
> +}
> +
>  struct key_store *
>  key_store_add_from_selector (struct key_store *ks, const char *selector)
>  {
>    CLEANUP_FREE_STRING_LIST char **fields =
>      guestfs_int_split_string (':', selector);

Reviewed-by: Richard W.M. Jones <rjones at redhat.com>


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org


More information about the Libguestfs mailing list