Skip to content

Commit dd2fc08

Browse files
committed
pf: fix possibe SCTP panic
While processing SCTP packets we can enqueue work for later, in the sctp_multihome_jobs queue. This deferred job includes a copy of the current struct pf_pdesc, which must contain a valid pcksum pointer (in case of NAT). However, jobs could be enqueued before we'd actually set this pointer in pf_setup_pdesc(). Set this pointer before we scan the SCTP chunk headers (and could enqueue deferred jobs.) While here sprinkle in a few more assertions to ensure we got this right. Reported-by: [email protected] MFC after: 3 days Sponsored by: Rubicon Communications, LLC ("Netgate")
1 parent eee36ff commit dd2fc08

File tree

1 file changed

+13
-6
lines changed

1 file changed

+13
-6
lines changed

sys/netpfil/pf/pf.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3364,7 +3364,7 @@ pf_change_ap(struct pf_pdesc *pd, struct pf_addr *a, u_int16_t *p,
33643364
u_int16_t po;
33653365
uint8_t u = pd->virtual_proto == IPPROTO_UDP;
33663366

3367-
MPASS(pd->pcksum);
3367+
MPASS(pd->pcksum != NULL);
33683368
if (pd->af == AF_INET) {
33693369
MPASS(pd->ip_sum);
33703370
}
@@ -7634,6 +7634,7 @@ pf_sctp_multihome_delayed(struct pf_pdesc *pd, struct pfi_kkif *kif,
76347634
nj->pd.m = j->pd.m;
76357635
nj->op = j->op;
76367636

7637+
MPASS(nj->pd.pcksum);
76377638
TAILQ_INSERT_TAIL(&pd->sctp_multihome_jobs, nj, next);
76387639
}
76397640
PF_SCTP_ENDPOINTS_UNLOCK();
@@ -7753,6 +7754,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
77537754
job->pd.m = pd->m;
77547755
job->op = op;
77557756

7757+
MPASS(job->pd.pcksum);
77567758
TAILQ_INSERT_TAIL(&pd->sctp_multihome_jobs, job, next);
77577759
break;
77587760
}
@@ -7786,6 +7788,7 @@ pf_multihome_scan(int start, int len, struct pf_pdesc *pd, int op)
77867788
job->pd.m = pd->m;
77877789
job->op = op;
77887790

7791+
MPASS(job->pd.pcksum);
77897792
TAILQ_INSERT_TAIL(&pd->sctp_multihome_jobs, job, next);
77907793
break;
77917794
}
@@ -10634,17 +10637,21 @@ pf_setup_pdesc(sa_family_t af, int dir, struct pf_pdesc *pd, struct mbuf **m0,
1063410637
REASON_SET(reason, PFRES_SHORT);
1063510638
return (-1);
1063610639
}
10637-
if (pf_scan_sctp(pd) != PF_PASS) {
10638-
*action = PF_DROP;
10639-
REASON_SET(reason, PFRES_SHORT);
10640-
return (-1);
10641-
}
10640+
1064210641
/*
1064310642
* Placeholder. The SCTP checksum is 32-bits, but
1064410643
* pf_test_state() expects to update a 16-bit checksum.
1064510644
* Provide a dummy value which we'll subsequently ignore.
10645+
* Do this before pf_scan_sctp() so any jobs we enqueue
10646+
* have a pcksum set.
1064610647
*/
1064710648
pd->pcksum = &pd->sctp_dummy_sum;
10649+
10650+
if (pf_scan_sctp(pd) != PF_PASS) {
10651+
*action = PF_DROP;
10652+
REASON_SET(reason, PFRES_SHORT);
10653+
return (-1);
10654+
}
1064810655
break;
1064910656
}
1065010657
case IPPROTO_ICMP: {

0 commit comments

Comments
 (0)