diff options
authorMichael Lotz <>2012-11-14 13:40:01 (GMT)
committerMichael Lotz <>2012-11-14 14:09:54 (GMT)
commitad53cd29cb74a329731b9e75938f547db4cb2272 (patch)
parent1a6f1cb937a4357f22a7947b333aa3d6faa3b78e (diff)
Fix iterator in hash_remove_current() to not skip elements.hrev44835
When iterator->current is NULL, hash_next() assumes we've reached the end of a bucket (linked list) and moves to the next one. Wehn the first element of a linked list was removed in hash_remove_current() iterator->current was set to NULL, causing the next call to hash_next() to skip over the rest of the list of that bucket. To fix this we now decrement iterator->bucket by one, so the next call to hash_next() correctly arrives at the new first element of the same bucket. Doing it this way avoids having to search backwards through the table to find the actual previous item. This caused modules to be skipped in module_init_post_boot_device() when normalizing module image paths so some of the module images ended up non-normalized. This could then cause images to be loaded a second time for modules that were part of an actually already loaded image. This setup is present for the PCI module with the pci/x86 submodule and would lead to a second copy of the PCI module image to be loaded but without being initialized, eventually leading to #8684. The affected module images were pretty much random, as it depended on the order in which they were loaded from the file system, in this case the boot floppy archive of the El-Torito boot part of ISO and anyboot images. The r1alpha4 release images unfortunately had the module files ordered in the archive just so that the PCI module image would be skipped, allowing #8684 to happen on many systems with MSI support. Since the block cache uses hash_remove_current() as well in some cases, it is possible that transactions in its list could've been skipped. Cursory testing didn't reveal this to be a usual case, and it is possible that in the pattern it is used there, the bug wouldn't be triggered at all. It's still possible that it caused rare misbehaviour though.
1 files changed, 7 insertions, 0 deletions
diff --git a/src/system/kernel/util/khash.cpp b/src/system/kernel/util/khash.cpp
index dded1ad..f218354 100644
--- a/src/system/kernel/util/khash.cpp
+++ b/src/system/kernel/util/khash.cpp
@@ -253,6 +253,13 @@ hash_remove_current(struct hash_table *table, struct hash_iterator *iterator)
} else {
table->table[index] = (struct hash_element *)NEXT(table,
+ // We need to rewind the bucket, as hash_next() advances to the
+ // next bucket when iterator->current is NULL. With this we
+ // basically move the iterator between the end of the last
+ // bucket and before the start of this one so hash_next()
+ // doesn't skip the rest of this bucket.
+ iterator->bucket--;