From 27da73f05f168c70ff1f3edaff3261e818b2c194 Mon Sep 17 00:00:00 2001 From: Enrico Joerns Date: Fri, 30 Aug 2024 08:09:17 +0200 Subject: [PATCH] src/hash_index: fix hash_index fd ownership handling When calling r_hash_index_open(), the ownership of the file descriptor provided should be handed over to the hash index and will be later closed in r_hash_index_free(). Thus, the file descriptor provided must not be freed by the calling code anymore. In r_hash_index_open_image() is was manually handled by setting data_fd = -1; after sucessfully opening the hash index. In generate_adaptive_data() however, this manual resetting of the pointer was missing and the fd was closed manually in later error handling. This could lead to cases where a double-call of g_close() on the same pointer happened. This was wrong anyway but results in a critical error in RAUC since glib version 2.75.0 which comes with a more pickier g_close() handling: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/2964 Since the need to manually set the fd to -1 seems error-prone anyway, we just change r_hash_index_open() to handle this itself by passing the fd as a pointer. Signed-off-by: Enrico Joerns --- include/hash_index.h | 8 ++++++-- src/bundle.c | 6 ++++-- src/hash_index.c | 19 ++++++++++--------- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/include/hash_index.h b/include/hash_index.h index a5f8d2a4a..93cb1ac3d 100644 --- a/include/hash_index.h +++ b/include/hash_index.h @@ -38,14 +38,18 @@ typedef struct { * If an existing hash index file is provided via 'hashes_filename', this will * be used instead of building a new index. * + * If creating the hash index succeeded, the ownership of the provided file + * descriptor will be transfered to the hash index by setting the provided file + * descriptor to -1. + * * @param label label for hash index (used for debugging/identification) - * @param data_fd open file descriptor of file to hash + * @param data_fd pointer to open file descriptor of file to hash * @param hashes_filename name of existing hash index file to use instead, or NULL * @param error return location for a GError, or NULL * * @return a newly allocated RaucHashIndex or NULL on error */ -RaucHashIndex *r_hash_index_open(const gchar *label, int data_fd, const gchar *hashes_filename, GError **error) +RaucHashIndex *r_hash_index_open(const gchar *label, int *data_fd, const gchar *hashes_filename, GError **error) G_GNUC_WARN_UNUSED_RESULT; /** diff --git a/src/bundle.c b/src/bundle.c index d15277567..851d399fa 100644 --- a/src/bundle.c +++ b/src/bundle.c @@ -413,7 +413,7 @@ static gboolean generate_adaptive_data(RaucManifest *manifest, const gchar *dir, return FALSE; } - index = r_hash_index_open("image", fd, NULL, &ierror); + index = r_hash_index_open("image", &fd, NULL, &ierror); if (!index) { g_propagate_prefixed_error( error, @@ -428,7 +428,9 @@ static gboolean generate_adaptive_data(RaucManifest *manifest, const gchar *dir, error, ierror, "Failed to write hash index for %s: ", image->filename); - g_close(fd, NULL); + if (fd >= 0) { + g_close(fd, NULL); + } return FALSE; } diff --git a/src/hash_index.c b/src/hash_index.c index 5c248fc40..820dc1d97 100644 --- a/src/hash_index.c +++ b/src/hash_index.c @@ -226,19 +226,19 @@ static void hash_index_prepare(RaucHashIndex *idx) idx->match_stats = r_stats_new(idx->label); } -RaucHashIndex *r_hash_index_open(const gchar *label, int data_fd, const gchar *hashes_filename, GError **error) +RaucHashIndex *r_hash_index_open(const gchar *label, int *data_fd, const gchar *hashes_filename, GError **error) { GError *ierror = NULL; g_autoptr(RaucHashIndex) idx = g_new0(RaucHashIndex, 1); g_return_val_if_fail(label, NULL); - g_return_val_if_fail(data_fd >= 0, NULL); + g_return_val_if_fail(data_fd || *data_fd >= 0, NULL); g_return_val_if_fail(error == NULL || *error == NULL, NULL); idx->label = g_strdup(label); - idx->data_fd = data_fd; + idx->data_fd = *data_fd; - idx->count = get_chunk_count(data_fd, &ierror); + idx->count = get_chunk_count(idx->data_fd, &ierror); if (!idx->count) { g_propagate_error(error, ierror); return NULL; @@ -273,7 +273,7 @@ RaucHashIndex *r_hash_index_open(const gchar *label, int data_fd, const gchar *h if (!idx->hashes) { g_message("Building new hash index for %s with %"G_GUINT32_FORMAT " chunks", label, idx->count); - idx->hashes = hash_file(data_fd, idx->count, &ierror); + idx->hashes = hash_file(idx->data_fd, idx->count, &ierror); if (!idx->hashes) { g_propagate_error(error, ierror); return NULL; @@ -282,6 +282,9 @@ RaucHashIndex *r_hash_index_open(const gchar *label, int data_fd, const gchar *h hash_index_prepare(idx); + /* transfer fd ownership */ + *data_fd = -1; + return g_steal_pointer(&idx); } @@ -346,12 +349,11 @@ RaucHashIndex *r_hash_index_open_slot(const gchar *label, const RaucSlot *slot, index_filename = g_build_filename(dir, "block-hash-index", NULL); /* r_hash_index_open handles missing index file */ - idx = r_hash_index_open(label, data_fd, index_filename, &ierror); + idx = r_hash_index_open(label, &data_fd, index_filename, &ierror); if (!idx) { g_propagate_error(error, ierror); goto out; } - data_fd = -1; /* belongs to idx now */ g_debug("opened hash index for slot %s as %s", slot->name, label); out: @@ -384,12 +386,11 @@ RaucHashIndex *r_hash_index_open_image(const gchar *label, const RaucImage *imag index_filename = g_strdup_printf("%s.block-hash-index", image->filename); - idx = r_hash_index_open(label, data_fd, index_filename, &ierror); + idx = r_hash_index_open(label, &data_fd, index_filename, &ierror); if (!idx) { g_propagate_error(error, ierror); goto out; } - data_fd = -1; /* belongs to idx now */ g_debug("opened hash index for image %s with index %s", image->filename, index_filename); out: