Skip to content

Commit

Permalink
pf: fix fragment hole count
Browse files Browse the repository at this point in the history 
Fragment reassembly finishes when no holes are left in the fragment
queue.  In certain overlap conditions, the hole counter was wrong
and pf(4) created an incomplete IP packet.  Before adjusting the
length, remove the overlapping fragment from the queue and insert
it again afterwards.  pf_frent_remove() and pf_frent_insert() adjust
the hole counter automatically.

bug reported and fix tested by Lucas Aubard with Johan Mazel, Gilles
Guette and Pierre Chifflier; OK claudio@

MFC after:	1 week
Obtained from:	OpenBSD, bluhm <bluhm@openbsd.org>, 9915416fe8
Sponsored by:	Rubicon Communications, LLC ("Netgate")

(cherry picked from commit 8b2feafb535d10a559b995c6fc2529715f927e2a)
  • Loading branch information
@kprovost @fichtner
kprovost authored and fichtner committed Feb 25, 2025
1 parent ebfe6da commit 5be39bc
Showing 1 changed file with 10 additions and 23 deletions.
33 changes: 10 additions & 23 deletions sys/netpfil/pf/pf_norm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,6 @@ pf_fillup_fragment(struct pf_fragment_cmp *key, struct pf_frent *frent,
struct pf_frent *after, *next, *prev;
struct pf_fragment *frag;
uint16_t total;
int old_index, new_index;

PF_FRAG_ASSERT();

Expand Down Expand Up @@ -661,32 +660,20 @@ pf_fillup_fragment(struct pf_fragment_cmp *key, struct pf_frent *frent,
uint16_t aftercut;

aftercut = frent->fe_off + frent->fe_len - after->fe_off;
DPFPRINTF(("adjust overlap %d\n", aftercut));
if (aftercut < after->fe_len) {
DPFPRINTF(("frag tail overlap %d", aftercut));
m_adj(after->fe_m, aftercut);
old_index = pf_frent_index(after);
/* Fragment may switch queue as fe_off changes */
pf_frent_remove(frag, after);
after->fe_off += aftercut;
after->fe_len -= aftercut;
new_index = pf_frent_index(after);
if (old_index != new_index) {
DPFPRINTF(("frag index %d, new %d\n",
old_index, new_index));
/* Fragment switched queue as fe_off changed */
after->fe_off -= aftercut;
after->fe_len += aftercut;
/* Remove restored fragment from old queue */
pf_frent_remove(frag, after);
after->fe_off += aftercut;
after->fe_len -= aftercut;
/* Insert into correct queue */
if (pf_frent_insert(frag, after, prev)) {
DPFPRINTF(
("fragment requeue limit exceeded\n"));
m_freem(after->fe_m);
uma_zfree(V_pf_frent_z, after);
/* There is not way to recover */
goto bad_fragment;
}
/* Insert into correct queue */
if (pf_frent_insert(frag, after, prev)) {
DPFPRINTF(("fragment requeue limit exceeded"));
m_freem(after->fe_m);
uma_zfree(V_pf_frent_z, after);
/* There is not way to recover */
goto free_fragment;
}
break;
}
Expand Down

0 comments on commit 5be39bc

Please sign in to comment.