From owner-freebsd-bugs@FreeBSD.ORG Tue Mar 28 10:30:16 2006 Return-Path: X-Original-To: freebsd-bugs@freebsd.org Delivered-To: freebsd-bugs@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 11C8A16A422; Tue, 28 Mar 2006 10:30:16 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.FreeBSD.org (Postfix) with ESMTP id 643EF43D49; Tue, 28 Mar 2006 10:30:13 +0000 (GMT) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id D6F3046BA0; Tue, 28 Mar 2006 05:30:12 -0500 (EST) Date: Tue, 28 Mar 2006 10:30:12 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: zhouyi zhou In-Reply-To: <20060328181002.1c8c5691.zhouyi04@ios.cn> Message-ID: <20060328102522.S19236@fledge.watson.org> References: <20060327184013.6d60173c.zhouyi04@ios.cn> <20060328095916.A19236@fledge.watson.org> <20060328181002.1c8c5691.zhouyi04@ios.cn> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: gnn@FreeBSD.org, freebsd-bugs@freebsd.org, bz@FreeBSD.org, trustedbsd-discuss@FreeBSD.org Subject: Re: settling serious conflicts between MAC and IPSEC X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Mar 2006 10:30:16 -0000 On Tue, 28 Mar 2006, zhouyi zhou wrote: > It is my pleasure, is any one willing to settle the mbuf without label > initialized problem in function ipfw_tick? if there is none, I am willing to > do it. I took a quick glance at that one, need to look at it some more. The tricky thing is figuring out what label to assign. ipfw_tick() appears to use send_pkt() to generate keeplives; this function is also used to generate RST's. In the RST case, we have existing MAC entry points to generate the label of a replying RST to a TCP segment, and should use that. In the case where a keepalive is spontaneously generated by the firewall rule, that's a little more tricky. One possibility is that ipfw needs to learn about associating MAC labels with IPFW dynamic rules. Another possibility is that IPFW should use the a label assigned for spontaneously generated packets. The former is certainly more complicated to implement, but is more what one actually wants, whereas the latter is easy to implement, but means that the keepalive might not actually be delivered to the end socket because the label might not match. If you have time to look at this, that would be great -- I'm pretty occupied for the next few days, and it would be very good to get a fix into the RELENG_5 and RELENG_6 trees before their respective releases in the next couple of weeks. I'm not entirely sure why ipfw2 is generating keepalives -- normally, it strikes me that that is something the two end hosts would do, not the intermediate firewall. Thanks, Robert N M Watson > On Tue, 28 Mar 2006 10:02:39 +0000 (GMT) > Robert Watson wrote: > >> >> On Mon, 27 Mar 2006, zhouyi zhou wrote: >> >>> High everyone, there exists a serious bug in function ipsec_copypkt(m) of >>> netinet6/ipsec.c in FreeBSD 5.4, FreeBSD 6.0 and FreeBSD 7.0 >>> >>> 3469 MGETHDR(mnew, M_DONTWAIT, MT_HEADER); >>> 3470 if (mnew == NULL) >>> 3471 goto fail; >>> 3472 mnew->m_pkthdr = n->m_pkthdr; >>> 3473 #if 0 >>> 3474 /* XXX: convert to m_tag or delete? */ >>> 3475 if (n->m_pkthdr.aux) { >>> 3476 mnew->m_pkthdr.aux = >>> 3477 m_copym(n->m_pkthdr.aux, >>> 3478 0, M_COPYALL, M_DONTWAIT); >>> 3479 } >>> 3480 #endif >>> 3481 M_MOVE_PKTHDR(mnew, n); >>> >>> On line 3472, mnew->m_pkthdr is assigned n->m_pkthdr, and on line 3481, in >>> function m_move_pkthdr, mnew's tag list will be delete (and the n's tag of >>> cause). This will cause system to crash. >>> >>> After commenting out line 3472, everything is OK. >> >> Thanks for this report! The M_MOVE_PKTHDR() should do all the necessary work, >> including copying the fields referenced in 3472, as well as handling existing >> m_tags right. I've attached a patch with your proposal, which looks and >> sounds good to me, and CC'd George and Bjoern in the hopes that one of them >> will give it a node of approval before I commit it -- hopefully we can get >> this MFC'd for 6.1-RELEASE. >> >> Robert N M Watson >> >> Index: ipsec.c >> =================================================================== >> RCS file: /home/ncvs/src/sys/netinet6/ipsec.c,v >> retrieving revision 1.43 >> diff -u -r1.43 ipsec.c >> --- ipsec.c 25 Jul 2005 12:31:42 -0000 1.43 >> +++ ipsec.c 28 Mar 2006 09:58:54 -0000 >> @@ -3469,15 +3469,6 @@ >> MGETHDR(mnew, M_DONTWAIT, MT_HEADER); >> if (mnew == NULL) >> goto fail; >> - mnew->m_pkthdr = n->m_pkthdr; >> -#if 0 >> - /* XXX: convert to m_tag or delete? */ >> - if (n->m_pkthdr.aux) { >> - mnew->m_pkthdr.aux = >> - m_copym(n->m_pkthdr.aux, >> - 0, M_COPYALL, M_DONTWAIT); >> - } >> -#endif >> M_MOVE_PKTHDR(mnew, n); >> } >> else { >> >