[XFRM][PATCH v5] Fix unexpected SA hard expiration after setting new date

[XFRM][PATCH v5] Fix unexpected SA hard expiration after setting new date

[Fan Du]

Hi, Dave

Hope v5 is better than previous ones :)
Any comments are really welcome!

Thanks


Changelog:
v1->v2
1) use xflags instead of creating new flags(suggested by Steffen Klassert)

v2->v3
1) fix email problem, and remove cc to myself(requested by David Miller)

v3->v4
1) fix typo when clearing XFRM_SOFT_EXPIRE(thanks for David Miller)
2) fix email problem, and remove cc to myself AGAIN!!!

v4->v5
1) remove unnecessary empty line (David Miller)


*Background*:
Once IPsec SAs are created between two peers, kernel setup a timer to monitor
two events: soft/hard expiration. However the timer handler use xtime to
caculate whether it's soft or hard expiration event.

normal code flow(hard expire time:100s, soft expire time:82s)

a) When new SAs created, xfrm_timer_handler is called one second
after its creation. At this point, calculate soft expire
interval(81s), setup the timer;

b) soft expire occur, rearm the timer with hard expire interval(18s)
then notify racoon2 about soft expire event. racoon2 will create
new SAs.

c) hard expire happen, notify racoon2 about it. racoon2 will delete
the old SAs.

*Scenario*:
Setting a new date before b),and after a) could result c) happens first,
As a result, old SAs is deleted before new ones are created. Normally
new SAs will be created by the next time networking traffic, but there
is a small time being when networking connection is down, this could
result in upper layer connections failed in tel comm area, thus it's
better to keep it strict in sequence.

*Workaround*:
set new time could happen:
1) before a), then SAs is updated with new time.
2) before b),and after a)
2a) When new SAs created, xfrm_timer_handler is called one second
after its creation. At this point, calculate soft expire
interval(81s), setup the timer;(set flag to mark next time should
be soft time expire)

<<---- new date comes

2b) soft expire occur, the calculation results in a hard time expire
event, but flag is set, so catch ya. Sync the addtime, and rearm
the timer with hard expire interval(18s), then notify racoon2
about soft expire event;

2c) hard expire happen, notify racoon2 about it;
so everything is in order.

3) after b), hard expire always happened anyway.

[PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[Fan Du]

After SA is setup, one timer is armed to detect soft/hard expiration,
however the timer handler uses xtime to do the math. This makes hard
expiration occurs first before soft expiration after setting new date
with big interval. As a result new child SA is deleted before rekeying
the new one.

Signed-off-by: Fan Du <fdu@windriver.com>
---
 include/net/xfrm.h    |    4 ++++
 net/xfrm/xfrm_state.c |   22 ++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index d9509eb..62b619e 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -213,6 +213,9 @@ struct xfrm_state {
 	struct xfrm_lifetime_cur curlft;
 	struct tasklet_hrtimer	mtimer;
 
+	/* used to fix curlft->add_time when changing date */
+	long		saved_tmo;
+
 	/* Last used time */
 	unsigned long		lastused;
 
@@ -238,6 +241,7 @@ static inline struct net *xs_net(struct xfrm_state *x)
 
 /* xflags - make enum if more show up */
 #define XFRM_TIME_DEFER	1
+#define XFRM_SOFT_EXPIRE 2
 
 enum {
 	XFRM_STATE_VOID,
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 5b228f9..fb64dc6 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -415,8 +415,17 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
 	if (x->lft.hard_add_expires_seconds) {
 		long tmo = x->lft.hard_add_expires_seconds +
 			x->curlft.add_time - now;
-		if (tmo <= 0)
-			goto expired;
+		if (tmo <= 0) {
+			if (x->xflags & XFRM_SOFT_EXPIRE) {
+				/* enter hard expire without soft expire first?!
+				 * setting a new date could trigger this.
+				 * workarbound: fix x->curflt.add_time by below:
+				 */
+				x->curlft.add_time = now - x->saved_tmo - 1;
+				tmo = x->lft.hard_add_expires_seconds - x->saved_tmo;
+			} else
+				goto expired;
+		}
 		if (tmo < next)
 			next = tmo;
 	}
@@ -433,10 +443,14 @@ static enum hrtimer_restart xfrm_timer_handler(struct hrtimer * me)
 	if (x->lft.soft_add_expires_seconds) {
 		long tmo = x->lft.soft_add_expires_seconds +
 			x->curlft.add_time - now;
-		if (tmo <= 0)
+		if (tmo <= 0) {
 			warn = 1;
-		else if (tmo < next)
+			x->xflags &= ~XFRM_SOFT_EXPIRE;
+		} else if (tmo < next) {
 			next = tmo;
+			x->xflags |= XFRM_SOFT_EXPIRE;
+			x->saved_tmo = tmo;
+		}
 	}
 	if (x->lft.soft_use_expires_seconds) {
 		long tmo = x->lft.soft_use_expires_seconds +
-- 
1.7.1

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: Fan Du <fdu@windriver.com>
Date: Tue, 31 Jul 2012 15:43:54 +0800

> After SA is setup, one timer is armed to detect soft/hard expiration,
> however the timer handler uses xtime to do the math. This makes hard
> expiration occurs first before soft expiration after setting new date
> with big interval. As a result new child SA is deleted before rekeying
> the new one.
> 
> Signed-off-by: Fan Du <fdu@windriver.com>

Applied.

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

You know what Fan Du, I'm extremely irritated about your email
situation.

Every time you post a patch, I reply, and I get this crap:

Diagnostic-Code: smtp; 5.1.0 - Unknown address error 550-'5.2.1 <fdu@windriver.com>... Mailbox disabled for this recipient' (delivery attempts: 0)

I've seen this at least 7 times, and this absolutely has to stop.

Otherwise I'm ignoring every patch you submit, it's as simple as
that.

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 02 Aug 2012 00:23:08 -0700 (PDT)

> 
> You know what Fan Du, I'm extremely irritated about your email
> situation.
> 
> Every time you post a patch, I reply, and I get this crap:

And this email triggered it too, what gives?

Re: [PATCH] [XFRM] Fix unexpected SA hard expiration after changing date

[Fan Du]

Hi, all

*Apologize* for all the trouble I brought to everyone, who gave me
advices/suggestions using fdu@windriver.com.
I'm truly sorry that all the inconvenience caused by my mistake.

fdu@windriver.com is obsolete!!!
I will use fan.du@windriver.com from now on.

Thanks


On 2012年08月02日 15:23, David Miller wrote:
>
> You know what Fan Du, I'm extremely irritated about your email
> situation.
>
> Every time you post a patch, I reply, and I get this crap:
>
> Diagnostic-Code: smtp; 5.1.0 - Unknown address error 550-'5.2.1<fdu@windriver.com>... Mailbox disabled for this recipient' (delivery attempts: 0)
>
> I've seen this at least 7 times, and this absolutely has to stop.
>
> Otherwise I'm ignoring every patch you submit, it's as simple as
> that.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

-- 

Love each day!
--fan