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 <[email protected]>

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;
 
 /**

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;
 				}
 

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,7 +349,7 @@ 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;
@@ -384,12 +387,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: