Merge pull request #3013 from cgwalters/more-analyzer-fixes

2023.6 coverity minor fixes

Makefile-tests.am

@@ -366,7 +366,7 @@ tests_test_bsdiff_CFLAGS = $(TESTS_CFLAGS)
 tests_test_bsdiff_LDADD = libbsdiff.la $(TESTS_LDADD)
 
 tests_test_otcore_CFLAGS = $(AM_CFLAGS) $(OT_INTERNAL_GIO_UNIX_CFLAGS) -I$(srcdir)/src/libotutil -I$(srcdir)/src/libotcore -I$(srcdir)/libglnx
-tests_test_otcore_LDADD = $(OT_INTERNAL_GIO_UNIX_LIBS) libotcore.la libglnx.la
+tests_test_otcore_LDADD = $(OT_INTERNAL_GIO_UNIX_LIBS) libotcore.la libglnx.la libotutil.la
 
 tests_test_checksum_SOURCES = \
 	src/libostree/ostree-core.c \

src/libostree/ostree-repo-commit.c

@@ -2850,14 +2850,6 @@ ostree_repo_write_content_finish (OstreeRepo *self, GAsyncResult *result, guchar
   return TRUE;
 }
 
-static GVariant *
-create_empty_gvariant_dict (void)
-{
-  GVariantBuilder builder;
-  g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{sv}"));
-  return g_variant_builder_end (&builder);
-}
-
 /**
  * ostree_repo_write_commit:
  * @self: Repo
@@ -2951,7 +2943,7 @@ ostree_repo_write_commit_with_time (OstreeRepo *self, const char *parent, const
     return FALSE;
 
   g_autoptr (GVariant) commit = g_variant_new (
-      "(@a{sv}@ay@a(say)sst@ay@ay)", new_metadata ? new_metadata : create_empty_gvariant_dict (),
+      "(@a{sv}@ay@a(say)sst@ay@ay)", new_metadata,
       parent ? ostree_checksum_to_bytes_v (parent) : ot_gvariant_new_bytearray (NULL, 0),
       g_variant_new_array (G_VARIANT_TYPE ("(say)"), NULL, 0), subject ? subject : "",
       body ? body : "", GUINT64_TO_BE (time),

src/libostree/ostree-sign-ed25519.c

@@ -415,17 +415,18 @@ ostree_sign_ed25519_add_pk (OstreeSign *self, GVariant *public_key, GError **err
   if (!_ostree_sign_ed25519_is_initialized (sign, error))
     return FALSE;
 
-  gpointer key = NULL;
+  g_autofree guint8 *key_owned = NULL;
+  const guint8 *key = NULL;
   gsize n_elements = 0;
 
   if (g_variant_is_of_type (public_key, G_VARIANT_TYPE_STRING))
     {
       const gchar *pk_ascii = g_variant_get_string (public_key, NULL);
-      key = g_base64_decode (pk_ascii, &n_elements);
+      key = key_owned = g_base64_decode (pk_ascii, &n_elements);
     }
   else if (g_variant_is_of_type (public_key, G_VARIANT_TYPE_BYTESTRING))
     {
-      key = (gpointer)g_variant_get_fixed_array (public_key, &n_elements, sizeof (guchar));
+      key = g_variant_get_fixed_array (public_key, &n_elements, sizeof (guchar));
     }
   else
     {
@@ -461,7 +462,7 @@ _ed25519_add_revoked (OstreeSign *self, GVariant *revoked_key, GError **error)
 
   const gchar *rk_ascii = g_variant_get_string (revoked_key, NULL);
   gsize n_elements = 0;
-  gpointer key = g_base64_decode (rk_ascii, &n_elements);
+  g_autofree guint8 *key = g_base64_decode (rk_ascii, &n_elements);
 
   if (!validate_length (n_elements, OSTREE_SIGN_ED25519_PUBKEY_SIZE, error))
     return glnx_prefix_error (error, "Incorrect ed25519 revoked key");

src/libotcore/otcore-prepare-root.c

@@ -82,12 +82,12 @@ otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error
     {
       if (strcmp (slot_suffix, "_a") == 0)
         {
-          *out_target = strdup (slot_a);
+          *out_target = g_strdup (slot_a);
           return TRUE;
         }
       else if (strcmp (slot_suffix, "_b") == 0)
         {
-          *out_target = strdup (slot_b);
+          *out_target = g_strdup (slot_b);
           return TRUE;
         }
       return glnx_throw (error, "androidboot.slot_suffix invalid: %s", slot_suffix);
@@ -98,11 +98,42 @@ otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error
    */
   if (proc_cmdline_has_key_starting_with (cmdline, "androidboot."))
     {
-      *out_target = strdup (slot_a);
+      *out_target = g_strdup (slot_a);
       return TRUE;
     }
 
   // Otherwise, fall back to the default `ostree=` kernel cmdline
   *out_target = otcore_find_proc_cmdline_key (cmdline, "ostree");
   return TRUE;
 }
+
+// Load a config file; if it doesn't exist, we return an empty configuration.
+// NULL will be returned if we caught an error.
+GKeyFile *
+otcore_load_config (int rootfs_fd, const char *filename, GError **error)
+{
+  // The path to the config file for this binary
+  static const char *const config_roots[] = { "usr/lib", "etc" };
+  g_autoptr (GKeyFile) ret = g_key_file_new ();
+
+  for (guint i = 0; i < G_N_ELEMENTS (config_roots); i++)
+    {
+      glnx_autofd int fd = -1;
+      g_autofree char *path = g_build_filename (config_roots[i], filename, NULL);
+      if (!ot_openat_ignore_enoent (rootfs_fd, path, &fd, error))
+        return NULL;
+      /* If the config file doesn't exist, that's OK */
+      if (fd == -1)
+        continue;
+
+      g_print ("Loading %s\n", path);
+
+      g_autofree char *buf = glnx_fd_readall_utf8 (fd, NULL, NULL, error);
+      if (!buf)
+        return NULL;
+      if (!g_key_file_load_from_data (ret, buf, -1, 0, error))
+        return NULL;
+    }
+
+  return g_steal_pointer (&ret);
+}

src/libotcore/otcore.h

@@ -46,6 +46,8 @@ gboolean otcore_validate_ed25519_signature (GBytes *data, GBytes *pubkey, GBytes
 char *otcore_find_proc_cmdline_key (const char *cmdline, const char *key);
 gboolean otcore_get_ostree_target (const char *cmdline, char **out_target, GError **error);
 
+GKeyFile *otcore_load_config (int rootfs, const char *filename, GError **error);
+
 // Our directory with transient state (eventually /run/ostree-booted should be a link to
 // /run/ostree/booted)
 #define OTCORE_RUN_OSTREE "/run/ostree"

src/switchroot/ostree-prepare-root.c

@@ -76,8 +76,6 @@
 #include "ot-keyfile-utils.h"
 #include "otcore.h"
 
-// The path to the config file for this binary
-const char *config_roots[] = { "/usr/lib", "/etc" };
 #define PREPARE_ROOT_CONFIG_PATH "ostree/prepare-root.conf"
 
 // This key is used by default if present in the initramfs to verify
@@ -106,35 +104,6 @@ const char *config_roots[] = { "/usr/lib", "/etc" };
 
 #include "ostree-mount-util.h"
 
-// Load our config file; if it doesn't exist, we return an empty configuration.
-// NULL will be returned if we caught an error.
-static GKeyFile *
-load_config (GError **error)
-{
-  g_autoptr (GKeyFile) ret = g_key_file_new ();
-
-  for (guint i = 0; i < G_N_ELEMENTS (config_roots); i++)
-    {
-      glnx_autofd int fd = -1;
-      g_autofree char *path = g_build_filename (config_roots[i], PREPARE_ROOT_CONFIG_PATH, NULL);
-      if (!ot_openat_ignore_enoent (AT_FDCWD, path, &fd, error))
-        return NULL;
-      /* If the config file doesn't exist, that's OK */
-      if (fd == -1)
-        continue;
-
-      g_print ("Loading %s\n", path);
-
-      g_autofree char *buf = glnx_fd_readall_utf8 (fd, NULL, NULL, error);
-      if (!buf)
-        return NULL;
-      if (!g_key_file_load_from_data (ret, buf, -1, 0, error))
-        return NULL;
-    }
-
-  return g_steal_pointer (&ret);
-}
-
 static bool
 sysroot_is_configured_ro (const char *sysroot)
 {
@@ -278,8 +247,8 @@ static void
 free_composefs_config (ComposefsConfig *config)
 {
   g_ptr_array_unref (config->pubkeys);
-  free (config->signature_pubkey);
-  free (config);
+  g_free (config->signature_pubkey);
+  g_free (config);
 }
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC (ComposefsConfig, free_composefs_config)
@@ -350,7 +319,14 @@ main (int argc, char *argv[])
     err (EXIT_FAILURE, "usage: ostree-prepare-root SYSROOT");
   const char *root_arg = argv[1];
 
-  g_autoptr (GKeyFile) config = load_config (&error);
+  // Since several APIs want to operate in terms of file descriptors, let's
+  // open the initramfs now.  Currently this is just used for the config parser.
+  glnx_autofd int initramfs_rootfs_fd = -1;
+  if (!glnx_opendirat (AT_FDCWD, "/", FALSE, &initramfs_rootfs_fd, &error))
+    errx (EXIT_FAILURE, "Failed to open /: %s", error->message);
+
+  g_autoptr (GKeyFile) config
+      = otcore_load_config (initramfs_rootfs_fd, PREPARE_ROOT_CONFIG_PATH, &error);
   if (!config)
     errx (EXIT_FAILURE, "Failed to parse config: %s", error->message);
 

tests/test-otcore.c

@@ -81,12 +81,61 @@ test_prepare_root_cmdline (void)
   free (g_steal_pointer (&target));
 }
 
+static void
+test_prepare_root_config (void)
+{
+  g_autoptr (GError) error = NULL;
+  g_auto (GLnxTmpDir) tmpdir = {
+    0,
+  };
+  g_assert (glnx_mkdtempat (AT_FDCWD, "/tmp/test-XXXXXX", 0777, &tmpdir, &error));
+  g_assert_no_error (error);
+
+  {
+    g_autoptr (GKeyFile) config = NULL;
+    g_auto (GStrv) keys = NULL;
+    config = otcore_load_config (tmpdir.fd, "ostree/someconfig.conf", &error);
+    g_assert (config);
+    keys = g_key_file_get_groups (config, NULL);
+    g_assert (keys && *keys == NULL);
+  }
+
+  g_assert (glnx_shutil_mkdir_p_at (tmpdir.fd, "usr/lib/ostree", 0755, NULL, NULL));
+  g_assert (glnx_file_replace_contents_at (tmpdir.fd, "usr/lib/ostree/someconfig.conf",
+                                           (guint8 *)"[foo]\nbar=baz", -1, 0, NULL, NULL));
+
+  {
+    g_autoptr (GKeyFile) config = NULL;
+    g_auto (GStrv) keys = NULL;
+    config = otcore_load_config (tmpdir.fd, "ostree/someconfig.conf", &error);
+    g_assert (config);
+    keys = g_key_file_get_groups (config, NULL);
+    g_assert (keys);
+    g_assert_cmpstr (*keys, ==, "foo");
+  }
+
+  g_assert (glnx_shutil_mkdir_p_at (tmpdir.fd, "etc/ostree", 0755, NULL, NULL));
+  g_assert (glnx_file_replace_contents_at (tmpdir.fd, "usr/lib/ostree/someconfig.conf",
+                                           (guint8 *)"[test]\nbar=baz", -1, 0, NULL, NULL));
+
+  {
+    g_autoptr (GKeyFile) config = NULL;
+    g_auto (GStrv) keys = NULL;
+    config = otcore_load_config (tmpdir.fd, "ostree/someconfig.conf", &error);
+    g_assert (config);
+    keys = g_key_file_get_groups (config, NULL);
+    g_assert (keys);
+    g_assert_cmpstr (*keys, ==, "test");
+  }
+}
+
 int
 main (int argc, char **argv)
 {
   g_test_init (&argc, &argv, NULL);
   otcore_ed25519_init ();
   g_test_add_func ("/ed25519", test_ed25519);
   g_test_add_func ("/prepare-root-cmdline", test_prepare_root_cmdline);
+  g_test_add_func ("/prepare-root-config", test_prepare_root_config);
   return g_test_run ();
 }