Hello Martin, *,
Please take a look at the chart and you'll notice that if
for any reason the order of the packet processing is like
illustrated we get a segfault in
src/libcharon/sa/tasks/child_rekey.c:244
since child_sa has already been nuked be the peer's delete
notification:
<snip>
242 /* disable close action for the redundand child */
243 child_sa =
other->child_create->get_child(other->child_create);
244 child_sa->set_close_action(child_sa, ACTION_NONE);
<snap>
Here's the chart:
max huckebein
rrq \ / rrq
\ /
\/
/\
/ \
< >
rrs \
\
\
\
\
> collision
/ detect
/ irq
/
/
/
irs <
\
\
\
\
>
collision X
detect X
irq !SEGFAULT!
c == child_sa
i == informational [delete]
r == rekey
rq == request
rs == response
Find the patch to solve this attached and consider it for inclusion.
Thank you,
Thomas
P.S.: No signature this time ;)
>From 30c6f22bf1ee32a1ed3d74e9c1f49b490477c7e8 Mon Sep 17 00:00:00 2001
From: Thomas Egerer <[email protected]>
Date: Mon, 2 Aug 2010 16:46:29 +0200
Subject: [PATCH] Do not touch child from collision if peer deleted it
---
src/libcharon/sa/tasks/child_rekey.c | 27 ++++++++++++++++++++++++---
1 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/src/libcharon/sa/tasks/child_rekey.c b/src/libcharon/sa/tasks/child_rekey.c
index fb3452e..c87a3ef 100644
--- a/src/libcharon/sa/tasks/child_rekey.c
+++ b/src/libcharon/sa/tasks/child_rekey.c
@@ -75,6 +75,15 @@ struct private_child_rekey_t {
* colliding task, may be delete or rekey
*/
task_t *collision;
+
+ /**
+ * Indicate that peer destroyed the redundant child from collision.
+ * This happens if a peer's delete notification for the redundant
+ * child gets processed before the rekey job. If so, we must not
+ * touch the child created in the collision since it points to
+ * memory already freed.
+ */
+ bool other_child_destroyed;
};
/**
@@ -239,9 +248,13 @@ static child_sa_t *handle_collision(private_child_rekey_t *this)
DBG1(DBG_IKE, "CHILD_SA rekey collision won, "
"deleting rekeyed child");
to_delete = this->child_sa;
- /* disable close action for the redundand child */
- child_sa = other->child_create->get_child(other->child_create);
- child_sa->set_close_action(child_sa, ACTION_NONE);
+ /* don't touch child other created, it has already been deleted */
+ if (!this->other_child_destroyed)
+ {
+ /* disable close action for the redundand child */
+ child_sa = other->child_create->get_child(other->child_create);
+ child_sa->set_close_action(child_sa, ACTION_NONE);
+ }
}
else
{
@@ -380,6 +393,13 @@ static void collide(private_child_rekey_t *this, task_t *other)
else if (other->get_type(other) == CHILD_DELETE)
{
child_delete_t *del = (child_delete_t*)other;
+ if (del->get_child(del) == this->child_create->get_child(this->child_create))
+ {
+ /* peer deletes redundant child created in collision */
+ this->other_child_destroyed = TRUE;
+ other->destroy(other);
+ return;
+ }
if (del == NULL || del->get_child(del) != this->child_sa)
{
/* not the same child => no collision */
@@ -466,6 +486,7 @@ child_rekey_t *child_rekey_create(ike_sa_t *ike_sa, protocol_id_t protocol,
this->spi = spi;
this->collision = NULL;
this->child_delete = NULL;
+ this->other_child_destroyed = FALSE;
return &this->public;
}
--
1.7.1
_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev
