oomd: Fix bug where we drop queued kill state on duplicate cgroup

oomd_cgroup_kill_mark() allocates a temporary OomdKillState and inserts
it into kill_states via set_ensure_put(). This is keyed by cgroup path.
When the same cgroup is already queued, set_ensure_put() dutifully
returns 0.

The function then returns with
_cleanup_(oomd_kill_state_removep) still armed, which eventually calls
oomd_kill_state_free().

oomd_kill_state_free() removes from kill_states by cgroup-path key, so
because this path already exists, it will remove the existing queued
kill state instead of just dropping the temporary object.

This is wrong and results in mistakenly drops the queued kill state on
duplicates.

This can happen when a cgroup is marked multiple times before the first
queued kill state is consumed. The result is lost kill-state tracking
and incorrect prekill/kill sequencing.

Handle r == 0 explicitly by freeing only the temporary object and
leaving the already queued state intact.

diff --git a/src/oom/oomd-util.c b/src/oom/oomd-util.c
index 340d2bb60e74ba242c5a8de6bc3e760df89f9ba7..97803a842840ebb0cac5abb7ed0e6fb49ffbba89 100644 (file)
--- a/src/oom/oomd-util.c
+++ b/src/oom/oomd-util.c
@@ -494,6 +494,14 @@ int oomd_cgroup_kill_mark(Manager *m, OomdCGroupContext *ctx) {
         r = set_ensure_put(&m->kill_states, &oomd_kill_state_hash_ops, ks);
         if (r < 0)
                 return log_oom_debug();
+        if (r == 0) {
+                /* The cgroup is already queued. Drop only this temporary object, because running the normal
+                 * cleanup path would remove by cgroup path key and could interfere with the existing queued
+                 * kill state. */
+                oomd_cgroup_context_unref(ks->ctx);
+                ks = mfree(ks);
+                return 0;
+        }
 
         r = oomd_prekill_hook(m, ks);
         if (r < 0)