From dc9eaef1e5143a0276d3fa144b7f295f4f01274a Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:50:54 -0400 Subject: [PATCH 1/4] tree: Fix `name` memory leak Coverity points out that we have a memory leak from `g_strdup(name)`. `insert_child_mtree()` takes a const char * and duplicates it. `name` can be passed directly to `insert_child_mtree()`. --- src/libostree/ostree-mutable-tree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 3a47de61ae..29f72a90fb 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -452,7 +452,7 @@ ostree_mutable_tree_ensure_parent_dirs (OstreeMutableTree *self, GPtrArray *spli invalidate_contents_checksum (subdir); next = ostree_mutable_tree_new (); ostree_mutable_tree_set_metadata_checksum (next, metadata_checksum); - insert_child_mtree (subdir, g_strdup (name), next); + insert_child_mtree (subdir, name, next); } subdir = next; From 20d5bc8453edf780b6a162a6568e211310211108 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:51:41 -0400 Subject: [PATCH 2/4] commit: Null terminate `target_buf` var Coverity points out that we are passing an unterminated string to sprintf(). Fix by using snprintf() which stores the content as a C string. --- src/libostree/ostree-repo-commit.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 7a898757cb..4d12d5ecb4 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -815,8 +815,8 @@ _try_clone_from_payload_link (OstreeRepo *self, OstreeRepo *dest_repo, const cha if (size < OSTREE_SHA256_STRING_LEN + _OSTREE_PAYLOAD_LINK_PREFIX_LEN) return glnx_throw (error, "invalid data size for %s", loose_path_buf); - sprintf (target_checksum, "%.2s%.62s", target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN, - target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN + 3); + snprintf (target_checksum, size, "%.2s%.62s", target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN, + target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN + 3); _ostree_loose_path (loose_path_target_buf, target_checksum, OSTREE_OBJECT_TYPE_FILE, self->mode); From d528083cae3492f9b9424f3c9830869af7b4cbd0 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:55:37 -0400 Subject: [PATCH 3/4] repo: Fix `dir_or_file_path` memory leak Coverity points out that we have a memory leak from `g_strdup(dir_or_file_path)`. Make the duplication of the string a temporary variable that is freed using `g_autofree`. --- src/libostree/ostree-repo-static-delta-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index dc57a2f03d..1a1b330952 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -409,7 +409,8 @@ ostree_repo_static_delta_execute_offline_with_signature (OstreeRepo *self, GFile return glnx_throw_errno_prefix (error, "openat(O_DIRECTORY)"); else { - g_autofree char *dir = dirname (g_strdup (dir_or_file_path)); + g_autofree char *tmpbuf = g_strdup (dir_or_file_path); + const char *dir = dirname (tmpbuf); basename = g_path_get_basename (dir_or_file_path); if (!glnx_opendirat (AT_FDCWD, dir, TRUE, &dfd, error)) From e99693c7871b593dd6703bd73344e80bb1d7e116 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Thu, 13 Jun 2024 14:41:27 -0400 Subject: [PATCH 4/4] prepare: Create global var for tmp_sysroot_etc Coverity points out that ""/sysroot.tmp/etc"" could be a copy-paste error. This is mistake from coverity, but to supress the warning, we create a global var, tmp_sysroot_etc, which replaces all instances of TMP_SYSROOT "/etc". --- src/switchroot/ostree-prepare-root.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 4ac4420272..2cb9f67ca9 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -537,6 +537,7 @@ main (int argc, char *argv[]) &etc_transient, &error)) errx (EXIT_FAILURE, "Failed to parse etc.transient value: %s", error->message); + static const char *tmp_sysroot_etc = TMP_SYSROOT "/etc"; if (etc_transient) { char *ovldir = "/run/ostree/transient-etc"; @@ -564,15 +565,15 @@ main (int argc, char *argv[]) g_autofree char *ovl_options = g_strdup_printf ("lowerdir=%s,upperdir=%s,workdir=%s", lowerdir, upperdir, workdir); - if (mount ("overlay", TMP_SYSROOT "/etc", "overlay", MS_SILENT, ovl_options) < 0) + if (mount ("overlay", tmp_sysroot_etc, "overlay", MS_SILENT, ovl_options) < 0) err (EXIT_FAILURE, "failed to mount transient etc overlayfs"); } else { /* Bind-mount /etc (at deploy path), and remount as writable. */ - if (mount ("etc", TMP_SYSROOT "/etc", NULL, MS_BIND | MS_SILENT, NULL) < 0) + if (mount ("etc", tmp_sysroot_etc, NULL, MS_BIND | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to prepare /etc bind-mount at /sysroot.tmp/etc"); - if (mount (TMP_SYSROOT "/etc", TMP_SYSROOT "/etc", NULL, MS_BIND | MS_REMOUNT | MS_SILENT, + if (mount (tmp_sysroot_etc, tmp_sysroot_etc, NULL, MS_BIND | MS_REMOUNT | MS_SILENT, NULL) < 0) err (EXIT_FAILURE, "failed to make writable /etc bind-mount at /sysroot.tmp/etc");