askrene: safer iterating over changing htable
What changed, and why it matters
This change fixes a subtle bug in Core Lightning's askrene plugin where adding items back into a hash table while looping through it could cause some items to be skipped. The fix collects items that need to be re-added and inserts them only after the loop finishes. The main risk is that the old code could leave stale routing constraints in place, potentially affecting payment routing decisions rather than directly stealing funds.
Review whether the same insert-during-iteration risk exists in the bias_hash and node_bias_hash loops in the same function and apply a consistent deferred-insertion pattern if needed. Otherwise, this patch should be included in routine maintenance.
Security signals we found
Hash table mutation during iteration
Potential skipped entries leading to stale routing constraints
Routing-layer correctness issue
No direct memory corruption or cryptographic weakness
Evidence from the diff
In plugins/askrene/layer.c, layer_trim_constraints iterates channel_intel_hash to remove expired constraints. When an entry still has unexpired members after trimming, it deletes and re-adds it. Re-insertion during iteration can resize/rehash the table and reposition entries, causing the iterator to skip some entries. The patch defers re-insertions to a temporary array and adds them after iteration completes. Similar patterns in the bias and node_bias loops are not changed, though the commit message only addresses this specific htable.
Changed components
plugins/askrene/layer.clayer_trim_constraints functionchannel_intel_hashInspect captured patch +17 / −4
diff --git a/plugins/askrene/layer.c b/plugins/askrene/layer.c
index cfc7225b..f3346ec0 100644
--- a/plugins/askrene/layer.c
+++ b/plugins/askrene/layer.c
@@ -1198,11 +1198,13 @@ size_t layer_trim_constraints(struct layer *layer, u64 cutoff)
size_t num_removed = 0;
struct channel_intel_hash_iter intelit;
const struct channel_intel *intelarr;
+ const struct channel_intel **modified_intelarr;
struct bias_hash_iter biasit;
struct bias *bias;
struct node_bias_hash_iter node_it;
struct node_bias *node_bias;
+ modified_intelarr = tal_arr(tmpctx, const struct channel_intel *, 0);
for (intelarr = channel_intel_hash_first(layer->channel_intels, &intelit);
intelarr;
intelarr = channel_intel_hash_next(layer->channel_intels, &intelit)) {
@@ -1219,7 +1221,9 @@ size_t layer_trim_constraints(struct layer *layer, u64 cutoff)
}
num_removed += count_old;
if(count_old){
- /* Remove from table before realloc! */
+ /* Remove from table before realloc!
+ * Removing elements from the table is safe during
+ * iteration. */
channel_intel_hash_del(layer->channel_intels, intelarr);
tal_arr_remove_range(&intelarr, 0, count_old);
@@ -1228,13 +1232,22 @@ size_t layer_trim_constraints(struct layer *layer, u64 cutoff)
if (tal_count(intelarr) == 0)
tal_free(intelarr);
else {
- /* Still has members, put it back. */
- channel_intel_hash_add(layer->channel_intels,
- intelarr);
+ /* Still has members, put it back.
+ * Inserting new elements in the table may
+ * modify the table's internal disposition of
+ * elements leading to missed elements during
+ * iteration. We add them later. */
+ tal_arr_expand(&modified_intelarr, intelarr);
}
}
}
+ for (size_t i = 0; i < tal_count(modified_intelarr); i++) {
+ channel_intel_hash_add(layer->channel_intels,
+ modified_intelarr[i]);
+ }
+ tal_free(modified_intelarr);
+
for (bias = bias_hash_first(layer->biases, &biasit); bias;
bias = bias_hash_next(layer->biases, &biasit)) {
if (bias->timestamp < cutoff) {
Why this scored 32/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.