Bug 121374 - [ipsec] SP refcnt increases with each packet in ipv6 with new IPSEC
Summary: [ipsec] SP refcnt increases with each packet in ipv6 with new IPSEC
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 7.0-RELEASE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Bjoern A. Zeeb
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-05 00:40 UTC by crahman
Modified: 2008-03-21 23:10 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description crahman 2008-03-05 00:40:00 UTC
Ok, this is actually probably more serious than I say, because when refcnt overflows KASSERT will cause some trouble.  But obviously no one is actually using ipv6 with IPSEC yet.

Anyway, if one creates an ipv6 association between two hosts with the new IPSEC,
each packet will increment the refcnt:

root# setkey -PD
hostA[any] hostB[any] any
        out ipsec
        esp/transport//use
        spid=3 seq=0 pid=1554
        refcnt=65

root# ping6 hostB
.. some packets go by

root# setkey -PD
hostA[any] hostB[any] any
        out ipsec
        esp/transport//use
        spid=3 seq=0 pid=1635
        refcnt=77

This problem does not occur with ipv4.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2008-03-05 05:16:41 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-net

Over to maintainer(s).
Comment 2 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-08 23:03:41 UTC
Hi,

can you try this patch and report back (might have an offset because
of the PR kern/121384 patch (apply that first)).

You can also fetch the patch from
http://sources.zabbadoz.net/freebsd/patchset/20080308-02-netipsec-sp-ref-pr121374.diff


Index: sys/netinet6/ip6_output.c
===================================================================
RCS file: /shared/mirror/FreeBSD/r/ncvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.114
diff -u -p -r1.114 ip6_output.c
--- sys/netinet6/ip6_output.c	2 Feb 2008 14:11:31 -0000	1.114
+++ sys/netinet6/ip6_output.c	8 Mar 2008 22:58:26 -0000
@@ -1080,6 +1085,10 @@ done:
  	} else if (ro_pmtu == &ip6route && ro_pmtu->ro_rt) {
  		RTFREE(ro_pmtu->ro_rt);
  	}
+#ifdef IPSEC
+	if (sp != NULL)
+		KEY_FREESP(&sp);
+#endif

  	return (error);


-- 
Bjoern A. Zeeb                                 bzeeb at Zabbadoz dot NeT
Software is harder than hardware  so better get it right the first time.
Comment 3 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-08 23:03:52 UTC
State Changed
From-To: open->feedback

Wait for feedback if the patch presented is fine. 


Comment 4 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-08 23:03:52 UTC
Responsible Changed
From-To: freebsd-net->bz

Take this. I have a patch.
Comment 5 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-13 07:06:59 UTC
On Wed, 12 Mar 2008, Cyrus Rahman wrote:

>> Synopsis: [ipsec] SP refcnt increases with each packet in ipv6 with new IPSEC
>>
>>  Wait for feedback if the patch presented is fine.
>>
>>  http://www.freebsd.org/cgi/query-pr.cgi?pr=121374
>
> Ok, I've tested this patch.  Exchanging packets through a policy works
> after a fashion, but after sending one packet the kernel deletes the
> policy, presumably because the refcnt goes to 0:
>
> hostB# setkey -DP
> hostA[any] hostB[any] any
>        in ipsec
>        esp/transport//require
>        spid=22 seq=1 pid=1037
>        refcnt=1
> hostB[any] hostA[any] any
>        out ipsec
>        esp/transport//require
>        spid=21 seq=0 pid=1037
>        refcnt=1
>
> hostB# ping6 hostA
> PING6(56=40+8+8 bytes) hostB --> hostA
> 16 bytes from hostA, icmp_seq=0 hlim=64 time=12.401 ms
> ^C
> --- hostA ping6 statistics ---
> 1 packets transmitted, 1 packets received, 0.0% packet loss
> round-trip min/avg/max/std-dev = 12.401/12.401/12.401/0.000 ms
>
> hostB# setkey -DP
> hostA[any] hostB[any] any
>        in ipsec
>        esp/transport//require
>        spid=22 seq=0 pid=1040
>        refcnt=1
>
> ****
>
> So the outbound policy is gone!

*sigh* I was already worried there was a problem with that while looking at
another IPsec path yesterday. I'll need to find another non-production
machine for testing things locally...

PS: I set the follow-up to gnats again so I won't lose track

-- 
Bjoern A. Zeeb                                 bzeeb at Zabbadoz dot NeT
Software is harder than hardware  so better get it right the first time.
Comment 6 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-13 17:44:59 UTC
State Changed
From-To: feedback->open

Feedback recv.ed. Needs more work.
Comment 7 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-13 21:50:13 UTC
Hi,

ok, here's a new patch. It seems to be working for me with this
and a similar transport//use policy to what you had.

Printing the refcnt before and after the KEY_FREESP in ip6_output:
[ip6_output:1091] sp=0xffffff0001fce700 refcnt=2-1
[ip6_output:1093] sp=0xffffff0001fce700 refcnt=1
[ip6_output:1091] sp=0xffffff0001fce700 refcnt=2-1
[ip6_output:1093] sp=0xffffff0001fce700 refcnt=1
[ip6_output:1091] sp=0xffffff0001fce700 refcnt=2-1
[ip6_output:1093] sp=0xffffff0001fce700 refcnt=1
[ip6_output:1091] sp=0xffffff0001fce700 refcnt=2-1
[ip6_output:1093] sp=0xffffff0001fce700 refcnt=1

I also checked the SPD before and after. The refcnt stayed at 1.

You can also fetch it from:
http://sources.zabbadoz.net/freebsd/patchset/20080313-02-netipsec-sp-ref-pr121374.diff

------------------------------------------------------------------------
Index: sys/netinet6/ip6_ipsec.c
===================================================================
RCS file: /shared/mirror/FreeBSD/r/ncvs/src/sys/netinet6/ip6_ipsec.c,v
retrieving revision 1.7
diff -u -p -r1.7 ip6_ipsec.c
--- sys/netinet6/ip6_ipsec.c	10 Dec 2007 16:03:38 -0000	1.7
+++ sys/netinet6/ip6_ipsec.c	13 Mar 2008 21:43:05 -0000
@@ -257,7 +257,7 @@ ip6_ipsec_output(struct mbuf **m, struct
  				 * NB: null pointer to avoid free at
  				 *     done: below.
  				 */
-				KEY_FREESP(sp), sp = NULL;
+				KEY_FREESP(sp), *sp = NULL;
  				/* XXX splx(s); */
  				goto done;
  			}
@@ -298,21 +298,16 @@ ip6_ipsec_output(struct mbuf **m, struct
  		}
  	}
  done:
-	if (sp != NULL)
-		if (*sp != NULL)
-			KEY_FREESP(sp);
  	return 0;
  do_ipsec:
  	return -1;
  bad:
-	if (sp != NULL)
-		if (*sp != NULL)
-			KEY_FREESP(sp);
  	return 1;
  #endif /* IPSEC */
  	return 0;
  }

+#if 0
  /*
   * Compute the MTU for a forwarded packet that gets IPSEC encapsulated.
   * Called from ip_forward().
@@ -363,7 +358,8 @@ ip6_ipsec_mtu(struct mbuf *m)
  #ifdef IPSEC
  		KEY_FREESP(&sp);
  #endif /* IPSEC */
-	}
+	} /* XXX ELSE MISSING ANYWAY */
  	return mtu;
  }
+#endif

Index: sys/netinet6/ip6_output.c
===================================================================
RCS file: /shared/mirror/FreeBSD/r/ncvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.114
diff -u -p -r1.114 ip6_output.c
--- sys/netinet6/ip6_output.c	2 Feb 2008 14:11:31 -0000	1.114
+++ sys/netinet6/ip6_output.c	13 Mar 2008 21:43:05 -0000
@@ -1080,6 +1086,10 @@ done:
  	} else if (ro_pmtu == &ip6route && ro_pmtu->ro_rt) {
  		RTFREE(ro_pmtu->ro_rt);
  	}
+#ifdef IPSEC
+	if (sp != NULL)
+		KEY_FREESP(&sp);
+#endif

  	return (error);

------------------------------------------------------------------------


-- 
Bjoern A. Zeeb                                 bzeeb at Zabbadoz dot NeT
Software is harder than hardware  so better get it right the first time.
Comment 8 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-13 21:50:44 UTC
State Changed
From-To: open->feedback

And there was a new patch and my world was fine, is yours as well?
Comment 9 crahman 2008-03-13 22:57:05 UTC
On Thu, Mar 13, 2008 at 3:50 PM, Bjoern A. Zeeb <bz@freebsd.org> wrote:
>
>  ok, here's a new patch. It seems to be working for me with this
>  and a similar transport//use policy to what you had.

I've tried this new patch and it solved the problem.

Thanks!
Comment 10 dfilter service freebsd_committer freebsd_triage 2008-03-14 11:55:10 UTC
bz          2008-03-14 11:55:05 UTC

  FreeBSD src repository

  Modified files:
    sys/netinet6         ip6_ipsec.c ip6_output.c 
  Log:
  Correct reference counting on the SP for outgoing IPv6 IPsec connections.
  
  PR:             121374
  Reported by:    Cyrus Rahman (crahman gmail.com)
  Tested by:      Cyrus Rahman (crahman gmail.com)
  MFC after:      5 days
  
  Revision  Changes    Path
  1.9       +1 -7      src/sys/netinet6/ip6_ipsec.c
  1.116     +4 -0      src/sys/netinet6/ip6_output.c
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 11 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-14 11:55:27 UTC
State Changed
From-To: feedback->patched

Patch comitted to HEAD. MFC in 5 days.
Comment 12 dfilter service freebsd_committer freebsd_triage 2008-03-21 23:08:44 UTC
bz          2008-03-21 23:08:36 UTC

  FreeBSD src repository

  Modified files:        (Branch: RELENG_7)
    sys/netinet6         ip6_ipsec.c ip6_output.c 
  Log:
  MFC: rev. 1.9 ip6_ipsec.c, 1.116 ip6_output.c
  
    Correct reference counting on the SP for outgoing IPv6 IPsec connections.
  
    PR:             121374
    Reported by:    Cyrus Rahman (crahman gmail.com)
    Tested by:      Cyrus Rahman (crahman gmail.com)
  
  Revision   Changes    Path
  1.6.2.2    +1 -7      src/sys/netinet6/ip6_ipsec.c
  1.109.2.5  +4 -0      src/sys/netinet6/ip6_output.c
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "cvs-all-unsubscribe@freebsd.org"
Comment 13 Bjoern A. Zeeb freebsd_committer freebsd_triage 2008-03-21 23:08:57 UTC
State Changed
From-To: patched->closed

Patch was MFCed. Thanks for reporting and testing.