From 5f1abe44f10bb6d8c7563104f5cf0f1511bb31cf Mon Sep 17 00:00:00 2001 From: Daniel Hartmeier Date: Fri, 29 Aug 2003 01:49:09 +0000 Subject: Fix three cases of potential accesses to free'd memory. At least one of them could be used to panic pf with scrub rules remotely. Found by Rob Pickering. ok frantzen@, henning --- sys/net/pf_norm.c | 152 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 78 insertions(+), 74 deletions(-) (limited to 'sys/net') diff --git a/sys/net/pf_norm.c b/sys/net/pf_norm.c index f0e16176bbd..02b81985f2e 100644 --- a/sys/net/pf_norm.c +++ b/sys/net/pf_norm.c @@ -1,4 +1,4 @@ -/* $OpenBSD: pf_norm.c,v 1.74 2003/08/22 21:50:34 david Exp $ */ +/* $OpenBSD: pf_norm.c,v 1.75 2003/08/29 01:49:08 dhartmei Exp $ */ /* * Copyright 2001 Niels Provos @@ -110,10 +110,10 @@ void pf_remove_fragment(struct pf_fragment *); void pf_flush_fragments(void); void pf_free_fragment(struct pf_fragment *); struct pf_fragment *pf_find_fragment(struct ip *, struct pf_frag_tree *); -struct mbuf *pf_reassemble(struct mbuf **, struct pf_fragment *, +struct mbuf *pf_reassemble(struct mbuf **, struct pf_fragment **, struct pf_frent *, int); struct mbuf *pf_fragcache(struct mbuf **, struct ip*, - struct pf_fragment *, int, int, int *); + struct pf_fragment **, int, int, int *); u_int16_t pf_cksum_fixup(u_int16_t, u_int16_t, u_int16_t); int pf_normalize_tcpopt(struct pf_rule *, struct mbuf *, struct tcphdr *, int); @@ -315,7 +315,7 @@ pf_remove_fragment(struct pf_fragment *frag) #define FR_IP_OFF(fr) ((ntohs((fr)->fr_ip->ip_off) & IP_OFFMASK) << 3) struct mbuf * -pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, +pf_reassemble(struct mbuf **m0, struct pf_fragment **frag, struct pf_frent *frent, int mff) { struct mbuf *m = *m0, *m2; @@ -327,33 +327,33 @@ pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, u_int16_t ip_len = ntohs(ip->ip_len) - ip->ip_hl * 4; u_int16_t max = ip_len + off; - KASSERT(frag == NULL || BUFFER_FRAGMENTS(frag)); + KASSERT(*frag == NULL || BUFFER_FRAGMENTS(*frag)); /* Strip off ip header */ m->m_data += hlen; m->m_len -= hlen; /* Create a new reassembly queue for this packet */ - if (frag == NULL) { - frag = pool_get(&pf_frag_pl, PR_NOWAIT); - if (frag == NULL) { + if (*frag == NULL) { + *frag = pool_get(&pf_frag_pl, PR_NOWAIT); + if (*frag == NULL) { pf_flush_fragments(); - frag = pool_get(&pf_frag_pl, PR_NOWAIT); - if (frag == NULL) + *frag = pool_get(&pf_frag_pl, PR_NOWAIT); + if (*frag == NULL) goto drop_fragment; } - frag->fr_flags = 0; - frag->fr_max = 0; - frag->fr_src = frent->fr_ip->ip_src; - frag->fr_dst = frent->fr_ip->ip_dst; - frag->fr_p = frent->fr_ip->ip_p; - frag->fr_id = frent->fr_ip->ip_id; - frag->fr_timeout = time.tv_sec; - LIST_INIT(&frag->fr_queue); + (*frag)->fr_flags = 0; + (*frag)->fr_max = 0; + (*frag)->fr_src = frent->fr_ip->ip_src; + (*frag)->fr_dst = frent->fr_ip->ip_dst; + (*frag)->fr_p = frent->fr_ip->ip_p; + (*frag)->fr_id = frent->fr_ip->ip_id; + (*frag)->fr_timeout = time.tv_sec; + LIST_INIT(&(*frag)->fr_queue); - RB_INSERT(pf_frag_tree, &pf_frag_tree, frag); - TAILQ_INSERT_HEAD(&pf_fragqueue, frag, frag_next); + RB_INSERT(pf_frag_tree, &pf_frag_tree, *frag); + TAILQ_INSERT_HEAD(&pf_fragqueue, *frag, frag_next); /* We do not have a previous fragment */ frep = NULL; @@ -364,7 +364,7 @@ pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, * Find a fragment after the current one: * - off contains the real shifted offset. */ - LIST_FOREACH(frea, &frag->fr_queue, fr_next) { + LIST_FOREACH(frea, &(*frag)->fr_queue, fr_next) { if (FR_IP_OFF(frea) > off) break; frep = frea; @@ -419,46 +419,47 @@ pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, insert: /* Update maximum data size */ - if (frag->fr_max < max) - frag->fr_max = max; + if ((*frag)->fr_max < max) + (*frag)->fr_max = max; /* This is the last segment */ if (!mff) - frag->fr_flags |= PFFRAG_SEENLAST; + (*frag)->fr_flags |= PFFRAG_SEENLAST; if (frep == NULL) - LIST_INSERT_HEAD(&frag->fr_queue, frent, fr_next); + LIST_INSERT_HEAD(&(*frag)->fr_queue, frent, fr_next); else LIST_INSERT_AFTER(frep, frent, fr_next); /* Check if we are completely reassembled */ - if (!(frag->fr_flags & PFFRAG_SEENLAST)) + if (!((*frag)->fr_flags & PFFRAG_SEENLAST)) return (NULL); /* Check if we have all the data */ off = 0; - for (frep = LIST_FIRST(&frag->fr_queue); frep; frep = next) { + for (frep = LIST_FIRST(&(*frag)->fr_queue); frep; frep = next) { next = LIST_NEXT(frep, fr_next); off += ntohs(frep->fr_ip->ip_len) - frep->fr_ip->ip_hl * 4; - if (off < frag->fr_max && + if (off < (*frag)->fr_max && (next == NULL || FR_IP_OFF(next) != off)) { DPFPRINTF(("missing fragment at %d, next %d, max %d\n", off, next == NULL ? -1 : FR_IP_OFF(next), - frag->fr_max)); + (*frag)->fr_max)); return (NULL); } } - DPFPRINTF(("%d < %d?\n", off, frag->fr_max)); - if (off < frag->fr_max) + DPFPRINTF(("%d < %d?\n", off, (*frag)->fr_max)); + if (off < (*frag)->fr_max) return (NULL); /* We have all the data */ - frent = LIST_FIRST(&frag->fr_queue); + frent = LIST_FIRST(&(*frag)->fr_queue); KASSERT(frent != NULL); if ((frent->fr_ip->ip_hl << 2) + off > IP_MAXPACKET) { DPFPRINTF(("drop: too big: %d\n", off)); - pf_free_fragment(frag); + pf_free_fragment(*frag); + *frag = NULL; return (NULL); } next = LIST_NEXT(frent, fr_next); @@ -480,11 +481,12 @@ pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, m_cat(m, m2); } - ip->ip_src = frag->fr_src; - ip->ip_dst = frag->fr_dst; + ip->ip_src = (*frag)->fr_src; + ip->ip_dst = (*frag)->fr_dst; /* Remove from fragment queue */ - pf_remove_fragment(frag); + pf_remove_fragment(*frag); + *frag = NULL; hlen = ip->ip_hl << 2; ip->ip_len = htons(off + hlen); @@ -512,7 +514,7 @@ pf_reassemble(struct mbuf **m0, struct pf_fragment *frag, } struct mbuf * -pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, +pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment **frag, int mff, int drop, int *nomem) { struct mbuf *m = *m0; @@ -522,41 +524,42 @@ pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, u_int16_t max = ip_len + off; int hosed = 0; - KASSERT(frag == NULL || !BUFFER_FRAGMENTS(frag)); + KASSERT(*frag == NULL || !BUFFER_FRAGMENTS(*frag)); /* Create a new range queue for this packet */ - if (frag == NULL) { - frag = pool_get(&pf_cache_pl, PR_NOWAIT); - if (frag == NULL) { + if (*frag == NULL) { + *frag = pool_get(&pf_cache_pl, PR_NOWAIT); + if (*frag == NULL) { pf_flush_fragments(); - frag = pool_get(&pf_cache_pl, PR_NOWAIT); - if (frag == NULL) + *frag = pool_get(&pf_cache_pl, PR_NOWAIT); + if (*frag == NULL) goto no_mem; } /* Get an entry for the queue */ cur = pool_get(&pf_cent_pl, PR_NOWAIT); if (cur == NULL) { - pool_put(&pf_cache_pl, frag); + pool_put(&pf_cache_pl, *frag); + *frag = NULL; goto no_mem; } pf_ncache++; - frag->fr_flags = PFFRAG_NOBUFFER; - frag->fr_max = 0; - frag->fr_src = h->ip_src; - frag->fr_dst = h->ip_dst; - frag->fr_p = h->ip_p; - frag->fr_id = h->ip_id; - frag->fr_timeout = time.tv_sec; + (*frag)->fr_flags = PFFRAG_NOBUFFER; + (*frag)->fr_max = 0; + (*frag)->fr_src = h->ip_src; + (*frag)->fr_dst = h->ip_dst; + (*frag)->fr_p = h->ip_p; + (*frag)->fr_id = h->ip_id; + (*frag)->fr_timeout = time.tv_sec; cur->fr_off = off; cur->fr_end = max; - LIST_INIT(&frag->fr_cache); - LIST_INSERT_HEAD(&frag->fr_cache, cur, fr_next); + LIST_INIT(&(*frag)->fr_cache); + LIST_INSERT_HEAD(&(*frag)->fr_cache, cur, fr_next); - RB_INSERT(pf_frag_tree, &pf_cache_tree, frag); - TAILQ_INSERT_HEAD(&pf_cachequeue, frag, frag_next); + RB_INSERT(pf_frag_tree, &pf_cache_tree, *frag); + TAILQ_INSERT_HEAD(&pf_cachequeue, *frag, frag_next); DPFPRINTF(("fragcache[%d]: new %d-%d\n", h->ip_id, off, max)); @@ -568,7 +571,7 @@ pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, * - off contains the real shifted offset. */ frp = NULL; - LIST_FOREACH(fra, &frag->fr_cache, fr_next) { + LIST_FOREACH(fra, &(*frag)->fr_cache, fr_next) { if (fra->fr_off > off) break; frp = fra; @@ -755,21 +758,22 @@ pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, pass: /* Update maximum data size */ - if (frag->fr_max < max) - frag->fr_max = max; + if ((*frag)->fr_max < max) + (*frag)->fr_max = max; /* This is the last segment */ if (!mff) - frag->fr_flags |= PFFRAG_SEENLAST; + (*frag)->fr_flags |= PFFRAG_SEENLAST; /* Check if we are completely reassembled */ - if ((frag->fr_flags & PFFRAG_SEENLAST) && - LIST_FIRST(&frag->fr_cache)->fr_off == 0 && - LIST_FIRST(&frag->fr_cache)->fr_end == frag->fr_max) { + if (((*frag)->fr_flags & PFFRAG_SEENLAST) && + LIST_FIRST(&(*frag)->fr_cache)->fr_off == 0 && + LIST_FIRST(&(*frag)->fr_cache)->fr_end == (*frag)->fr_max) { /* Remove from fragment queue */ DPFPRINTF(("fragcache[%d]: done 0-%d\n", h->ip_id, - frag->fr_max)); - pf_free_fragment(frag); + (*frag)->fr_max)); + pf_free_fragment(*frag); + *frag = NULL; } return (m); @@ -778,8 +782,8 @@ pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, *nomem = 1; /* Still need to pay attention to !IP_MF */ - if (!mff && frag) - frag->fr_flags |= PFFRAG_SEENLAST; + if (!mff && *frag != NULL) + (*frag)->fr_flags |= PFFRAG_SEENLAST; m_freem(m); return (NULL); @@ -787,15 +791,15 @@ pf_fragcache(struct mbuf **m0, struct ip *h, struct pf_fragment *frag, int mff, drop_fragment: /* Still need to pay attention to !IP_MF */ - if (!mff && frag) - frag->fr_flags |= PFFRAG_SEENLAST; + if (!mff && *frag != NULL) + (*frag)->fr_flags |= PFFRAG_SEENLAST; if (drop) { /* This fragment has been deemed bad. Don't reass */ - if ((frag->fr_flags & PFFRAG_DROP) == 0) + if (((*frag)->fr_flags & PFFRAG_DROP) == 0) DPFPRINTF(("fragcache[%d]: dropping overall fragment\n", h->ip_id)); - frag->fr_flags |= PFFRAG_DROP; + (*frag)->fr_flags |= PFFRAG_DROP; } m_freem(m); @@ -905,12 +909,12 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct ifnet *ifp, u_short *reason) /* Might return a completely reassembled mbuf, or NULL */ DPFPRINTF(("reass frag %d @ %d-%d\n", h->ip_id, fragoff, max)); - *m0 = m = pf_reassemble(m0, frag, frent, mff); + *m0 = m = pf_reassemble(m0, &frag, frent, mff); if (m == NULL) return (PF_DROP); - if (frag && (frag->fr_flags & PFFRAG_DROP)) + if (frag != NULL && (frag->fr_flags & PFFRAG_DROP)) goto drop; h = mtod(m, struct ip *); @@ -939,7 +943,7 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct ifnet *ifp, u_short *reason) goto bad; } - *m0 = m = pf_fragcache(m0, h, frag, mff, + *m0 = m = pf_fragcache(m0, h, &frag, mff, (r->rule_flag & PFRULE_FRAGDROP) ? 1 : 0, &nomem); if (m == NULL) { if (nomem) @@ -955,7 +959,7 @@ pf_normalize_ip(struct mbuf **m0, int dir, struct ifnet *ifp, u_short *reason) goto no_mem; m_tag_prepend(m, mtag); } - if (frag && (frag->fr_flags & PFFRAG_DROP)) + if (frag != NULL && (frag->fr_flags & PFFRAG_DROP)) goto drop; goto fragment_pass; } -- cgit v1.2.3