Page MenuHomeFreeBSD

pf: Make pf_get_translation() more expressive
AcceptedPublic

Authored by markj on Fri, Jun 21, 3:06 PM.

Details

Reviewers
kp
Summary

Currently pf_get_translation() returns a pointer to a matching
nat/rdr/binat rule, or NULL if no rule was matched or an error occurred
while applying the translation. That is, we don't distinguish between
errors and the lack of a matching rule. This, if an error (e.g., a
memory allocation failure or a state conflict) occurs, we simply handle
the packet as if no translation rule was present. This is not
desireable.

Make pf_get_translation() return the matching rule as an out-param and
instead return a reason code which indicates whether there was no
translation rule, or there was a translation rule and we failed to apply
it, or there was a translation rule and we applied it successfully.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 58335
Build 55223: arc lint + arc unit

Event Timeline

markj requested review of this revision.Fri, Jun 21, 3:06 PM
sys/netpfil/pf/pf.c
4517

This bails us out before we SLIST_INIT(&match_rules);, so on transerror we panic in the match_rules cleanup.

Easy enough to fix, and that init should have been at the top of the function already:

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 41889a37d0a4..bca447c40b25 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -4442,6 +4442,8 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,

        PF_RULES_RASSERT();

+       SLIST_INIT(&match_rules);
+
        if (inp != NULL) {
                INP_LOCK_ASSERT(inp);
                pd->lookup.uid = inp->inp_cred->cr_uid;
@@ -4666,7 +4668,6 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,
                pd->nat_rule = nr;
        }

-       SLIST_INIT(&match_rules);
        while (r != NULL) {
                pf_counter_u64_add(&r->evaluations, 1);
                if (pfi_kkif_match(r->kif, kif) == r->ifnot)

Initialize match_rules earlier.

This revision is now accepted and ready to land.Mon, Jun 24, 8:12 PM