attrs: preserve array counts on replacement

subject_add() and object_add() always incremented cnt after writing a slot. Replacing an existing subject or object attribute therefore made cnt larger than the number of occupied slots, which trips the DEBUG sanity checker as array corruption.

Teach both add paths to treat cnt as an occupied-slot count. They now compute the destination slot once, free the previous slot payload when replacing it, and increment cnt only when filling an empty slot.

Add an event_test regression that replaces EXE and PATH attributes and verifies cnt stays at one and the new attributes are cached. Verification: ./configure --with-debug --disable-shared --without-rpm --without-deb; make -j32; make -C src/tests -j32 check TESTS=event_test; make -j32 check.

Signed-off-by: Matt Jolly <kangie@gentoo.org>

Author: Kangie

src/library/object.c

@@ -78,6 +78,7 @@ object_attr_t *object_access(const o_array *a, object_type_t t)
 int object_add(o_array *a, const object_attr_t *obj)
 {
 	object_attr_t *newnode;
+	object_attr_t **slot;
 
 	if (a == NULL || a->obj == NULL)
 		return 1;
@@ -95,8 +96,14 @@ int object_add(o_array *a, const object_attr_t *obj)
 	} else
 		return 1;
 
-	a->obj[obj->type - OBJ_START] = newnode;
-	a->cnt++;
+	slot = &a->obj[obj->type - OBJ_START];
+	if (*slot) {
+		free((*slot)->o);
+		free(*slot);
+	} else
+		a->cnt++;
+
+	*slot = newnode;
 
 	return 0;
 }

src/library/subject.c

@@ -85,6 +85,7 @@ subject_attr_t *subject_access(const s_array *a, subject_type_t t)
 int subject_add(s_array *a, const subject_attr_t *subj)
 {
 	subject_attr_t* newnode;
+	subject_attr_t **slot;
 	subject_type_t t;
 
 	if (a == NULL || a->subj == NULL)
@@ -118,8 +119,19 @@ int subject_add(s_array *a, const subject_attr_t *subj)
 	} else
 		return 1;
 
-	a->subj[t - SUBJ_START] = newnode;
-	a->cnt++;
+	slot = &a->subj[t - SUBJ_START];
+	if (*slot) {
+		subject_attr_t *old = *slot;
+
+		if (old->type == GID || old->type == UID)
+			attr_set_destroy(old->set);
+		else if (old->type >= COMM)
+			free(old->str);
+		free(old);
+	} else
+		a->cnt++;
+
+	*slot = newnode;
 	sanity_check_array(a, "subject_add 2");
 
 	return 0;

src/tests/event_test.c

@@ -671,6 +671,58 @@ static int init_caches(unsigned int subj_size, unsigned int obj_size)
 	return init_event_system(&cfg);
 }
 
+/*
+ * test_attribute_replacement_preserves_count - verify subject/object array
+ * replacement keeps cnt as an occupied-slot count.
+ * Returns 0 on success, test error code otherwise.
+ */
+static int test_attribute_replacement_preserves_count(void)
+{
+	s_array subjects;
+	o_array objects;
+	subject_attr_t old_exe = { .type = EXE, .str = strdup("/old-exe") };
+	subject_attr_t new_exe = { .type = EXE, .str = strdup("/new-exe") };
+	object_attr_t old_path = { .type = PATH, .o = strdup("/old-path") };
+	object_attr_t new_path = { .type = PATH, .o = strdup("/new-path") };
+	subject_attr_t *cached_subject;
+	object_attr_t *cached_object;
+
+	CHECK(old_exe.str && new_exe.str && old_path.o && new_path.o, 130,
+	      "[ERROR:130] failed to allocate replacement attributes");
+	CHECK(subject_create(&subjects) == 0, 131,
+	      "[ERROR:131] failed to allocate subject array");
+	CHECK(object_create(&objects) == 0, 132,
+	      "[ERROR:132] failed to allocate object array");
+
+	CHECK(subject_add(&subjects, &old_exe) == 0, 133,
+	      "[ERROR:133] failed to add original subject attribute");
+	CHECK(subject_add(&subjects, &new_exe) == 0, 134,
+	      "[ERROR:134] failed to replace subject attribute");
+	CHECK(subjects.cnt == 1, 135,
+	      "[ERROR:135] subject replacement changed slot count");
+	cached_subject = subject_access(&subjects, EXE);
+	CHECK(cached_subject != NULL, 136,
+	      "[ERROR:136] replacement subject attribute missing");
+	CHECK(strcmp(cached_subject->str, "/new-exe") == 0, 137,
+	      "[ERROR:137] replacement subject attribute not cached");
+
+	CHECK(object_add(&objects, &old_path) == 0, 138,
+	      "[ERROR:138] failed to add original object attribute");
+	CHECK(object_add(&objects, &new_path) == 0, 139,
+	      "[ERROR:139] failed to replace object attribute");
+	CHECK(objects.cnt == 1, 140,
+	      "[ERROR:140] object replacement changed slot count");
+	cached_object = object_access(&objects, PATH);
+	CHECK(cached_object != NULL, 141,
+	      "[ERROR:141] replacement object attribute missing");
+	CHECK(strcmp(cached_object->o, "/new-path") == 0, 142,
+	      "[ERROR:142] replacement object attribute not cached");
+
+	subject_clear(&subjects);
+	object_clear(&objects);
+	return 0;
+}
+
 /*
  * test_attr_lookup_metrics - verify request, miss, dependency, and reset data.
  * Returns 0 on success, test error code otherwise.
@@ -1122,6 +1174,10 @@ int main(void)
 	if (rc)
 		return rc;
 
+	rc = test_attribute_replacement_preserves_count();
+	if (rc)
+		return rc;
+
 	rc = test_attr_lookup_metrics();
 	if (rc)
 		return rc;