Skip to content

Commit

Permalink
src/hash_index: fix hash_index fd ownership handling
Browse files Browse the repository at this point in the history
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 <ejo@pengutronix.de>
  • Loading branch information
ejoerns committed Aug 30, 2024
1 parent 893187f commit 27da73f
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 13 deletions.
8 changes: 6 additions & 2 deletions include/hash_index.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
6 changes: 4 additions & 2 deletions src/bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}

Expand Down
19 changes: 10 additions & 9 deletions src/hash_index.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 27da73f

Please sign in to comment.