settling serious conflicts between MAC and IPSEC

[Robert Watson]

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 {