From owner-freebsd-net  Tue Nov 14 13: 1:47 2000
Delivered-To: freebsd-net@freebsd.org
Received: from mail.gmx.net (pop.gmx.net [194.221.183.20])
	by hub.freebsd.org (Postfix) with SMTP id E77D337B4FE
	for <freebsd-net@freebsd.org>; Tue, 14 Nov 2000 13:01:39 -0800 (PST)
Received: (qmail 21222 invoked by uid 0); 14 Nov 2000 21:01:38 -0000
Received: from p3e9bc345.dip.t-dialin.net (HELO forge.local) (62.155.195.69)
  by mail.gmx.net (mail04) with SMTP; 14 Nov 2000 21:01:38 -0000
Received: from thomas by forge.local with local (Exim 3.12 #1 (Debian))
	id 13vnCf-0000I7-00; Tue, 14 Nov 2000 22:01:17 +0100
Date: Tue, 14 Nov 2000 22:01:17 +0100
To: freebsd-net@freebsd.org
Cc: bmilekic@dsuper.net
Subject: bug in bridging/dummynet code - PR kern/19551
Message-ID: <20001114220117.A732@forge.local>
Mail-Followup-To: tmoestl@gmx.net, freebsd-net@freebsd.org,
	bmilekic@dsuper.net
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
From: Thomas Moestl <tmoestl@gmx.net>
Sender: owner-freebsd-net@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.org

Hi,

I think I have spotted a bug in the bridge/dummynet code that might be 
responsible for some panics people have reported recently - see e.g.
PR kern/19551.
PR kern/21534 seems related and are probably about the same thing,
PR kern/19488  goes in the same direction.

Bosko, I'm CCing this to you because the PR is currently assigned to
you.

Here is the relevant section of code from netinet/ip_dummynet.c:402:

#ifdef BRIDGE
	case DN_TO_BDG_FWD : {
	    struct mbuf *m = (struct mbuf *)pkt ;
	    struct ether_header hdr;

	    if (m->m_len < ETHER_HDR_LEN
	      && (m = m_pullup(m, ETHER_HDR_LEN)) == NULL) {
		m_freem(m);
		break;
	    }
	    bcopy(mtod(m, struct ether_header *), &hdr, ETHER_HDR_LEN);
	    m_adj(m, ETHER_HDR_LEN);
	    bdg_forward(&m, &hdr, pkt->ifp);
	    if (m)
		m_freem(m);
	    }
	    break ;
#endif

Now, pkt is a malloc()ed structure, not an mbuf! Calling m_pullup() on it
seems defective, at least because m_free may be called in m_pullup(),
which leaks kernel memory if the freed structure is not an mbuf.
And of course, the ethernet header should be in the mbuf in pkt->dn_m.
Should it be:

#ifdef BRIDGE
	case DN_TO_BDG_FWD : {
	    struct mbuf *m = (struct mbuf *)pkt ;
	    struct ether_header hdr;

	    if (pkt->dn_m->m_len < ETHER_HDR_LEN
	      && (pkt->dn_m = m_pullup(pkt->dn_m, ETHER_HDR_LEN)) == NULL) {
		m_freem(pkt->dn_m);
		break;
	    }
	    bcopy(mtod(pkt->dn_m, struct ether_header *), &hdr, ETHER_HDR_LEN);
	    m_adj(pkt->dn_m, ETHER_HDR_LEN);
	    bdg_forward(&m, &hdr, pkt->ifp);
	    if (m)
		/* bdg_format will put pkt->dn_m into mbuf into m in our case */
		m_freem(m);
	    }
	    break ;
#endif

Hmm, maybe I'm wrong here, but that seems odd to me. Please enlighten
me! Unfortunetly, I have no machine I could use to test it at the moment.

I just wanted to ask before I add this to the PR.

Sorry if I was wrong,
	- Thomas




To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message