Page MenuHomeFreeBSD

pf: Make pf_get_translation() more expressive
ClosedPublic

Authored by markj on Jun 21 2024, 3:06 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Nov 7, 9:50 AM
Unknown Object (File)
Wed, Nov 6, 6:11 PM
Unknown Object (File)
Wed, Oct 30, 5:13 PM
Unknown Object (File)
Mon, Oct 28, 7:11 AM
Unknown Object (File)
Sat, Oct 26, 1:05 AM
Unknown Object (File)
Oct 7 2024, 1:08 PM
Unknown Object (File)
Oct 6 2024, 10:17 PM
Unknown Object (File)
Oct 6 2024, 6:40 PM

Details

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 58281
Build 55169: arc lint + arc unit

Event Timeline

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

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.Jun 24 2024, 8:12 PM
This revision was automatically updated to reflect the committed changes.