From owner-freebsd-net Fri Jun 7 13:16:56 2002 Delivered-To: freebsd-net@freebsd.org Received: from tesla.distributel.net (nat.MTL.distributel.NET [66.38.181.24]) by hub.freebsd.org (Postfix) with ESMTP id 7208337B404 for ; Fri, 7 Jun 2002 13:16:52 -0700 (PDT) Received: (from bmilekic@localhost) by tesla.distributel.net (8.11.6/8.11.6) id g57KFSi88174; Fri, 7 Jun 2002 16:15:28 -0400 (EDT) (envelope-from bmilekic@unixdaemons.com) Date: Fri, 7 Jun 2002 16:15:28 -0400 From: Bosko Milekic To: Archie Cobbs Cc: freebsd-net@FreeBSD.ORG Subject: Re: m->m_pkthdr.header Message-ID: <20020607161528.A88123@unixdaemons.com> References: <200206071955.g57JtrJ65814@arch20m.dellroad.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <200206071955.g57JtrJ65814@arch20m.dellroad.org>; from archie@dellroad.org on Fri, Jun 07, 2002 at 12:55:53PM -0700 Sender: owner-freebsd-net@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org On Fri, Jun 07, 2002 at 12:55:53PM -0700, Archie Cobbs wrote: > Hi, > > I'd like to get rid of this mbuf field "m->m_pkthdr.header". > Here are my reasons: > > - It's broken. Consider this code: > > copy = m_copypacket(m); > m_freem(m); Indeed, this is a bug in m_copypacket(). It should copy the data over, but again, admittedly, this is a very dangerous practise! Ideally, what ought to happen is that the copying over needs to be done by the code calling m_copypacket(), right after the m_copypacket() call but before the m_freem() call. The reason to do it from the caller code and not from m_copypacket() is that there is no reasonable way to determine, from m_copypacket(), whether or not one of our header/mbuf pointers points to a data inside the mbuf or whether it is safe to do the shallow pointer copy. > Typically m->m_pkthdr.header points into the data area of 'm'. > So now copy->m_pkthdr.header points at the same data, which has > already been freed and possibly reused. > > - It is an obscure thing most people don't know about and is > ripe for causing hard to find bugs. > > - It's only used in two places, (a) for secret communication between > the oltr driver and if_ether.c, and (b) as a temporary variable > in ip_input.c: > > $ find . -type f -print | xargs grep -w m_pkthdr.header > ./contrib/dev/oltr/if_oltr.c: m0->m_pkthdr.header = (void *)th; > ./netinet/if_ether.c: th = (struct iso88025_header *)m->m_pkthdr.header; > ./netinet/ip_input.c: m->m_pkthdr.header = ip; > ./netinet/ip_input.c:#define GETIP(m) ((struct ip*)((m)->m_pkthdr.header)) > > My proposal: > > - Replace the obfuscating GETIP() macro in ip_input.c with a variable. > - Rejigger the oltr driver to pass its "secret" information using > an auxillary mbuf instead of m->m_pkthdr.header. > > Any comments/objections? None from here. :-) Just the standard pointer, which you probably already know: -CURRENT first, please. > Thanks, > -Archie > > __________________________________________________________________________ > Archie Cobbs * Packet Design * http://www.packetdesign.com Regards, -- Bosko Milekic bmilekic@unixdaemons.com bmilekic@FreeBSD.org To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-net" in the body of the message