onas_ht_insert: fix issue causing active bckts to be unlinked from ht->head chain

[sharkautarch]

Whenever there was a hash collision, onas_ht_insert would try to reinsert the pointer to a preexisting bckt into the ht->head list of buckets, even though that bucket would have already been put into the list when it was first initialized w/ onas_bucket_init().
This can cause a break in the ht->head chain where some active bckts can not be accessed via iterating via bckt = ht->head+bckt = bckt->next, due to setting bckt->next to NULL.

This causes issues when trying to process OnAccessExcludePath, where iterating over the linked list of bckts currently causes some directories to not be excluded when they should

Here's a simple diagram to illustrate how onas_ht_insert is currently broken w/ hash collisions:

Lets say we first insert two bckts w/ onas_ht_insert, let B1 be the first bckt, & B2 be the second bckt:
let H: ht->head, T: ht->tail -->: link/bckt->next, <--: bckt-prev
first insertion:

         H --> B1
         T --> B1
NULL <-- B1 --> NULL

second insertion:

         H --> B1
         T --> B2
NULL <-- B1 --> B2
  B1 <-- B2 --> NULL

Normally, the third insertion for B3 would seem pretty intuitive:

         H --> B1
         T --> B3
NULL <-- B1 --> B2
  B1 <-- B2 --> B3
  B2 <-- B3 --> NULL

Now lets consider what happens if, for the third insertion, instead of a new bucket, B3, we try inserting B1 again:

T: B2
T->next = B1
B1->prev = T
B1->next = NULL
T = B1

         H --> B1
         T --> B1
  B2 <-- B1 --> NULL
  B1 <-- B2 --> B1

Now we can no longer access bckt B2 via bckt = ht->head & bckt = bckt->next, which, as mentioned, is what is used to iterate over each bucket for OnAccessExcludePath:

clamav/clamonacc/inotif/inotif.c

Lines 534 to 554 in 9fe785d

	struct onas_bucket *ob = ddd_ht->head;
	/* Iterate through the activated buckets to find matched paths */
	while (ob != NULL) {
	struct onas_element *oe = ob->head;
	while (oe != NULL) {
	if (match_regex(oe->key, pt->strarg)) {
	if (onas_ht_get(ddd_ht, oe->key, oe->klen, NULL) == CL_SUCCESS) {
	char *oe_key = cli_safer_strdup(oe->key);
	if (onas_ht_rm_hierarchy(ddd_ht, oe->key, oe->klen, 0)) {
	logg(LOGG_ERROR, "ClamInotif: can't exclude '%s'\n", oe_key);
	free(oe_key);
	return NULL;
	} else {
	logg(LOGG_INFO, "ClamInotif: excluding '%s' (and all sub-directories)\n", oe_key);
	free(oe_key);
	}
	}
	}
	oe = oe->next;
	}
	ob = ob->next;

Guard against this issue by only inserting the bckt if this is a newly initialized bckt

[Copilot]

Pull request overview

Fixes a bug in clamonacc’s inotify hash table where hash collisions could cause an already-active bucket to be re-linked into ht->head/ht->tail, breaking the activated-bucket linked list traversal (impacting OnAccessExcludePath processing).

Changes:

• Track whether a bucket was newly initialized during onas_ht_insert.
• Only append a bucket to the activated-bucket chain when it is newly created.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[val-ms]

Re: the force push: I fixed super minor formatting issues to resolve our format checks, and added detail to the commit message.

Your change looks great. Thank you for solving this. I can that you put a lot of thought into it. I really appreciated the diagrams, they helped a lot with review.

[sharkautarch]

@val-ms Thanks for the review!
:)