From a8c3c3da97b3f36b4c9c05fec2319ac83ab6646d Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:48:52 -0400 Subject: [PATCH 01/10] checksum: Null terminate buffer Coverity points out that `buffer` is an unterminated string. g_checksum_update() expects a null-terminated string. --- src/libostree/ostree-checksum-input-stream.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-checksum-input-stream.c b/src/libostree/ostree-checksum-input-stream.c index f06a95d89e..582cc13b98 100644 --- a/src/libostree/ostree-checksum-input-stream.c +++ b/src/libostree/ostree-checksum-input-stream.c @@ -132,7 +132,11 @@ ostree_checksum_input_stream_read (GInputStream *stream, void *buffer, gsize cou res = g_input_stream_read (fself->base_stream, buffer, count, cancellable, error); if (res > 0) - g_checksum_update (self->priv->checksum, buffer, res); + { + guchar *char_buffer = (guchar *)buffer; + char_buffer[res] = '\0'; + g_checksum_update (self->priv->checksum, (const guchar *)char_buffer, res); + } return res; } From 20a333cd21c18f6c66301ddf572051c8dcb66244 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:50:54 -0400 Subject: [PATCH 02/10] tree: autofree `name` var Coverity points out that we have a memory leak from `g_strdup(name)` --- src/libostree/ostree-mutable-tree.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-mutable-tree.c b/src/libostree/ostree-mutable-tree.c index 3a47de61ae..d8b65adf36 100644 --- a/src/libostree/ostree-mutable-tree.c +++ b/src/libostree/ostree-mutable-tree.c @@ -452,7 +452,8 @@ 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); + g_autofree char *name_copy = g_strdup (name); + insert_child_mtree (subdir, name_copy, next); } subdir = next; From f120b7b9211ba885b251ae461b94eefb490f09e7 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:51:41 -0400 Subject: [PATCH 03/10] commit: Null terminate `target_buf` var Coverity points out that we are passing an unterminated string to sprintf(). --- src/libostree/ostree-repo-commit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libostree/ostree-repo-commit.c b/src/libostree/ostree-repo-commit.c index 7a898757cb..7eef7f7a8d 100644 --- a/src/libostree/ostree-repo-commit.c +++ b/src/libostree/ostree-repo-commit.c @@ -815,6 +815,7 @@ _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); + target_buf[size] = '\0'; sprintf (target_checksum, "%.2s%.62s", target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN, target_buf + _OSTREE_PAYLOAD_LINK_PREFIX_LEN + 3); From d049c1f98f210303032cde436195b1493204d601 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:52:55 -0400 Subject: [PATCH 04/10] repo: Return copy of local var `ret` Coverity points out that we are returning `stbuf` which is a local variable. --- src/libostree/ostree-repo-libarchive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index 65a309335f..f25eceff09 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -244,7 +244,7 @@ aic_get_final_path (OstreeRepoArchiveImportContext *ctx, const char *path, GErro char *ret = ctx->opts->translate_pathname (ctx->repo, &stbuf, path, ctx->opts->translate_pathname_user_data); if (ret) - return ret; + return g_strdup (ret); /* Fall through */ } else if (ctx->opts->use_ostree_convention) From 22b77f142e55b9659381a0fbd014ecda5f23ef27 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:54:12 -0400 Subject: [PATCH 05/10] repo: Free actual pointer instead of stolen pointer Coverity points out that ostree_repo_finder_result_free() is freeing an incorrect pointer `(gpointer)g_steal_pointer(&results->pdata[i])`. Since we are not returning this value, we shouldn't need to transfer ownership of the pointer. Instead, we should free the actual pointer. --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 9989415fa2..fe3c8b077a 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -5952,7 +5952,7 @@ find_remotes_cb (GObject *obj, GAsyncResult *async_result, gpointer user_data) { g_debug ("%s: Omitting remote ‘%s’ from results as none of its refs are new enough.", G_STRFUNC, result->remote->name); - ostree_repo_finder_result_free (g_steal_pointer (&g_ptr_array_index (results, i))); + ostree_repo_finder_result_free (g_ptr_array_index (results, i)); continue; } } From 383f908f7f1851db39228c7245486b5e40cb105a Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:55:00 -0400 Subject: [PATCH 06/10] repo: Autofree `ret_rollsum` Coverity points out that we have a memory leak from `ret_rollsum`. --- src/libostree/ostree-repo-static-delta-compilation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 54e4801fe8..d8c6785916 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -608,7 +608,7 @@ try_content_rollsum (OstreeRepo *repo, DeltaOpts opts, const char *from, const c (unsigned long long)matches->match_size); } - ContentRollsum *ret_rollsum = g_new0 (ContentRollsum, 1); + g_autofree ContentRollsum *ret_rollsum = g_new0 (ContentRollsum, 1); ret_rollsum->from_checksum = g_strdup (from); ret_rollsum->matches = g_steal_pointer (&matches); ot_transfer_out_value (out_rollsum, &ret_rollsum); From b42ef62c050b1690bee8fb363fccf0200ce08958 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:55:37 -0400 Subject: [PATCH 07/10] repo: Autofree `dir_or_file_path` Coverity points out that we have a memory leak from g_strdup(dir_or_file_path). --- 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..ba2e9d46de 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 *dir_or_file_path_copy = g_strdup (dir_or_file_path); + g_autofree char *dir = dirname (dir_or_file_path_copy); basename = g_path_get_basename (dir_or_file_path); if (!glnx_opendirat (AT_FDCWD, dir, TRUE, &dfd, error)) From 81125ff18f55d94c089d971f5b24e9fd15729f73 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:56:28 -0400 Subject: [PATCH 08/10] repo: Autofree contents of `input_mfile` Coverity points out that we have memory leak from g_mapped_file_get_contents(input_mfile). --- src/libostree/ostree-repo-static-delta-processing.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index 94876fa448..5b4a960f79 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -423,10 +423,11 @@ dispatch_bspatch (OstreeRepo *repo, StaticDeltaExecutionState *state, GCancellab struct bspatch_stream stream; stream.read = bspatch_read; stream.opaque = &opaque; - if (bspatch ((const guint8 *)g_mapped_file_get_contents (input_mfile), - g_mapped_file_get_length (input_mfile), buf, state->content_size, &stream) + g_autofree const guint8 *old = (const guint8 *)g_mapped_file_get_contents (input_mfile); + if (bspatch (old, g_mapped_file_get_length (input_mfile), buf, + state->content_size, &stream) < 0) - return glnx_throw (error, "bsdiff patch failed"); + return glnx_throw (error, "bsdiff patch failed"); if (!_ostree_repo_bare_content_write (repo, &state->content_out, buf, state->content_size, cancellable, error)) From 377dc62f56c6a9e6eea51dd995f5cf466d3672ae Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:56:55 -0400 Subject: [PATCH 09/10] sysroot: Autofree `devpath` Coverity points out that we have a memory leak from g_strdup(devpath). --- src/libostree/ostree-sysroot.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index af9e07c1ab..78809a0db8 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -2243,7 +2243,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, OstreeDeployment *deploym deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT) : _ostree_sysroot_get_runstate_path ( deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT); - g_autofree char *devpath_parent = dirname (g_strdup (devpath)); + g_autofree char *devpath_copy = g_strdup (devpath); + g_autofree char *devpath_parent = dirname (devpath_copy); if (!glnx_shutil_mkdir_p_at (AT_FDCWD, devpath_parent, 0755, cancellable, error)) return FALSE; From 4b0fec2ff45f839107d0def4012a962894f587a1 Mon Sep 17 00:00:00 2001 From: Luke Yang Date: Wed, 12 Jun 2024 15:58:08 -0400 Subject: [PATCH 10/10] prepare: Change mount `etc` to absolute path Coverity points out that ""/sysroot.tmp/etc"" could be a copy-paste error. --- src/switchroot/ostree-prepare-root.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c index 4ac4420272..06260b5c4f 100644 --- a/src/switchroot/ostree-prepare-root.c +++ b/src/switchroot/ostree-prepare-root.c @@ -570,7 +570,7 @@ main (int argc, char *argv[]) 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, NULL)