[Bug 287254] kernel panic sys/netpfil/pf/pf.c:pf_return() NULL pointer dereference

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=287254

            Bug ID: 287254
           Summary: kernel panic sys/netpfil/pf/pf.c:pf_return() NULL
                    pointer dereference
           Product: Base System
           Version: 14.3-STABLE
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: pherman@frenchfries.net

Hi,

running 14.3-STABLE with mpd5/PPPoE + pf. About once a week I get a kernel
panic at the following line:
  sys/netpfil/pf/pf.c:3478 in pf_return()

                if (pd->sport)
                        *pd->sport = sk->port[pd->sidx];

Because sk is NULL the kernel panics.  The reason for this is because
pf_return() is called by pf_test_rule() as can be seen here:
  sys/netpfil/pf/pf.c:5068

                sk = nk = NULL;
                if (action != PF_PASS) {
                        pd->act.log |= PF_LOG_FORCE;
                        if (action == PF_DROP &&
                            (r->rule_flag & PFRULE_RETURN))
                                pf_return(r, nr, pd, sk, off, m, th, kif,
                                    bproto_sum, bip_sum, hdrlen, &reason,
                                    pd->act.rtableid);
                        return (action);

As you can see, pf_return() is blatantly being called with a NULL sk, and
pf_return() has no provision to deal with a NULL sk. pf_return() seems to just
assume that sk is not NULL.

I've been running with following patch to pf_return() now for a few weeks now
with no apparent problems:

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 8201d3e24db7..98eae102b623 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -3475,9 +3475,9 @@ pf_return(struct pf_krule *r, struct pf_krule *nr, struct
pf_pdesc *pd,
        if (nr != NULL) {
                PF_ACPY(saddr, &pd->osrc, pd->af);
                PF_ACPY(daddr, &pd->odst, pd->af);
-               if (pd->sport)
+               if (pd->sport && sk)
                        *pd->sport = sk->port[pd->sidx];
-               if (pd->dport)
+               if (pd->dport && sk)
                        *pd->dport = sk->port[pd->didx];
                if (pd->proto_sum)
                        *pd->proto_sum = bproto_sum;

But because I'm not familiar with the pf code and what is actually going on
here, I don't profess that this is any kind of "solution", not to mention that
this is most likely a problem that needs to be fixed upstream.

Anyone more familiar than me that wants to have a closer look?

-- 
You are receiving this mail because:
You are the assignee for the bug.