Hello,

I've just realized my suggestion [1] to lteo@ was not complete. The single
atomic_cas() used as I've suggested is not sufficient measure. The code
should also do a non-atomic test to check, whether the rule is not expired yet.

the non-atomic check deals with scenario, where competing thread arrives
to atomic_cas() well before the current thread.

OK?

thanks and
regards
sashan

[1] https://marc.info/?l=openbsd-bugs&m=156341942417148&w=2

--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/net/pf.c b/sys/net/pf.c
index b858c43963b..b001f185665 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -3963,8 +3963,9 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm, 
struct pf_state **sm,
                 * insert an expired rule to gcl.
                 */
                rule_flag = r->rule_flag;
-               if (atomic_cas_uint(&r->rule_flag, rule_flag,
-                   rule_flag | PFRULE_EXPIRED) == rule_flag) {
+               if (((rule_flag & PFRULE_EXPIRED) == 0) &&
+                   atomic_cas_uint(&r->rule_flag, rule_flag,
+                       rule_flag | PFRULE_EXPIRED) == rule_flag) {
                        r->exptime = time_second;
                        SLIST_INSERT_HEAD(&pf_rule_gcl, r, gcle);
                }

Reply via email to