Skip to content

Commit

Permalink
pull: Fix theoretical checksum collision for metadata fetches
Browse files Browse the repository at this point in the history
I was making some other changes in this code, and noticed that we were adding
checksums without object types into the same hash table for metadata. We should
*never* do this with both metadata content objects, since in theory a content
object could have the same hash as metadata.

I don't actually think it's possible in practice for pure metadata to collide,
since they have different structures, but let's do this anyways since it's
conceptually right.

Closes: #651
Approved by: giuseppe
  • Loading branch information
cgwalters authored and rh-atomic-bot committed Jan 19, 2017
1 parent 2f71136 commit b28b785
Showing 1 changed file with 8 additions and 8 deletions.
16 changes: 8 additions & 8 deletions src/libostree/ostree-repo-pull.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ typedef struct {
GHashTable *commit_to_depth; /* Maps commit checksum maximum depth */
GHashTable *scanned_metadata; /* Maps object name to itself */
GHashTable *requested_metadata; /* Maps object name to itself */
GHashTable *requested_content; /* Maps object name to itself */
GHashTable *requested_content; /* Maps checksum to itself */
guint n_outstanding_metadata_fetches;
guint n_outstanding_metadata_write_requests;
guint n_outstanding_content_fetches;
Expand Down Expand Up @@ -1267,7 +1267,7 @@ scan_one_metadata_object_c (OtPullData *pull_data,
if (g_hash_table_lookup (pull_data->scanned_metadata, object))
return TRUE;

is_requested = g_hash_table_lookup (pull_data->requested_metadata, tmp_checksum) != NULL;
is_requested = g_hash_table_lookup (pull_data->requested_metadata, object) != NULL;
if (!ostree_repo_has_object (pull_data->repo, objtype, tmp_checksum, &is_stored,
cancellable, error))
goto out;
Expand All @@ -1292,10 +1292,9 @@ scan_one_metadata_object_c (OtPullData *pull_data,

if (!is_stored && !is_requested)
{
char *duped_checksum = g_strdup (tmp_checksum);
gboolean do_fetch_detached;

g_hash_table_add (pull_data->requested_metadata, duped_checksum);
g_hash_table_add (pull_data->requested_metadata, g_variant_ref (object));

do_fetch_detached = (objtype == OSTREE_OBJECT_TYPE_COMMIT);
enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, do_fetch_detached, FALSE);
Expand Down Expand Up @@ -1467,10 +1466,11 @@ process_one_static_delta_fallback (OtPullData *pull_data,
{
if (OSTREE_OBJECT_TYPE_IS_META (objtype))
{
if (!g_hash_table_lookup (pull_data->requested_metadata, checksum))
g_autoptr(GVariant) objname = ostree_object_name_serialize (checksum, objtype);
if (!g_hash_table_lookup (pull_data->requested_metadata, objname))
{
gboolean do_fetch_detached;
g_hash_table_add (pull_data->requested_metadata, checksum);
g_hash_table_add (pull_data->requested_metadata, g_variant_ref (objname));

do_fetch_detached = (objtype == OSTREE_OBJECT_TYPE_COMMIT);
enqueue_one_object_request (pull_data, checksum, objtype, NULL, do_fetch_detached, FALSE);
Expand Down Expand Up @@ -2451,8 +2451,8 @@ ostree_repo_pull_with_options (OstreeRepo *self,
(GDestroyNotify)g_variant_unref, NULL);
pull_data->requested_content = g_hash_table_new_full (g_str_hash, g_str_equal,
(GDestroyNotify)g_free, NULL);
pull_data->requested_metadata = g_hash_table_new_full (g_str_hash, g_str_equal,
(GDestroyNotify)g_free, NULL);
pull_data->requested_metadata = g_hash_table_new_full (ostree_hash_object_name, g_variant_equal,
(GDestroyNotify)g_variant_unref, NULL);
if (dir_to_pull != NULL || dirs_to_pull != NULL)
{
pull_data->dirs = g_ptr_array_new_with_free_func (g_free);
Expand Down

0 comments on commit b28b785

Please sign in to comment.