Fixed a race condition when querying stats of a child_sa in different order.

diff --git a/src/charon/plugins/stroke/stroke_list.c b/src/charon/plugins/stroke/stroke_list.c
index 591fd3c..795e1ed 100644 (file)
--- a/src/charon/plugins/stroke/stroke_list.c
+++ b/src/charon/plugins/stroke/stroke_list.c
@@ -146,8 +146,7 @@ static void log_ike_sa(FILE *out, ike_sa_t *ike_sa, bool all)
  */
 static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all)
 {
-       u_int32_t rekey, now = time(NULL);
-       u_int32_t use_in, use_out;
+       time_t use_in, use_out, rekey, now = time(NULL);
        u_int64_t bytes_in, bytes_out;
        proposal_t *proposal;
        child_cfg_t *config = child_sa->get_config(child_sa);
@@ -207,26 +206,18 @@ static void log_child_sa(FILE *out, child_sa_t *child_sa, bool all)
                                }
                        }
 
-                       bytes_in = child_sa->get_usebytes(child_sa, TRUE);
+                       child_sa->get_usestats(child_sa, TRUE, &use_in, &bytes_in);
                        fprintf(out, ", %lu bytes_i", bytes_in);
-                       if (bytes_in)
+                       if (use_in)
                        {
-                               use_in = child_sa->get_usetime(child_sa, TRUE);
-                               if (use_in)
-                               {
-                                       fprintf(out, " (%ds ago)", now - use_in);
-                               }
+                               fprintf(out, " (%ds ago)", now - use_in);
                        }
 
-                       bytes_out = child_sa->get_usebytes(child_sa, FALSE);
+                       child_sa->get_usestats(child_sa, FALSE, &use_out, &bytes_out);
                        fprintf(out, ", %lu bytes_o", bytes_out);
-                       if (bytes_out)
+                       if (use_out)
                        {
-                               use_out = child_sa->get_usetime(child_sa, FALSE);
-                               if (use_out)
-                               {
-                                       fprintf(out, " (%ds ago)", now - use_out);
-                               }
+                               fprintf(out, " (%ds ago)", now - use_out);
                        }
                        fprintf(out, ", rekeying ");
                        

diff --git a/src/charon/sa/child_sa.c b/src/charon/sa/child_sa.c
index 8021c74..574dcb8 100644 (file)
--- a/src/charon/sa/child_sa.c
+++ b/src/charon/sa/child_sa.c
@@ -380,7 +380,7 @@ static enumerator_t* create_policy_enumerator(private_child_sa_t *this)
  * are available, and NOT_SUPPORTED if the kernel interface does not support
  * quering the usebytes.
  */
-static bool update_usebytes(private_child_sa_t *this, bool inbound)
+static status_t update_usebytes(private_child_sa_t *this, bool inbound)
 {
        status_t status = FAILED;
        u_int64_t bytes;
@@ -427,19 +427,14 @@ static bool update_usebytes(private_child_sa_t *this, bool inbound)
 }
 
 /**
- * Implementation of child_sa_t.get_usetime
+ * updates the cached usetime
  */
-static u_int32_t get_usetime(private_child_sa_t *this, bool inbound)
+static void update_usetime(private_child_sa_t *this, bool inbound)
 {
        enumerator_t *enumerator;
        traffic_selector_t *my_ts, *other_ts;
        u_int32_t last_use = 0;
        
-       if (update_usebytes(this, inbound) == FAILED)
-       {       /* no SPI or no traffic since last update */
-               return inbound ? this->my_usetime : this->other_usetime;
-       }
-       
        enumerator = create_policy_enumerator(this);
        while (enumerator->enumerate(enumerator, &my_ts, &other_ts))
        {
@@ -479,16 +474,26 @@ static u_int32_t get_usetime(private_child_sa_t *this, bool inbound)
        {
                this->other_usetime = last_use;
        }
-       return last_use;
 }
 
 /**
- * Implementation of child_sa_t.get_usebytes
+ * Implementation of child_sa_t.get_usestats
  */
-static u_int64_t get_usebytes(private_child_sa_t *this, bool inbound)
+static void get_usestats(private_child_sa_t *this, bool inbound,
+                                                time_t *time, u_int64_t *bytes)
 {
-       update_usebytes(this, inbound);
-       return inbound ? this->my_usebytes : this->other_usebytes;
+       if (update_usebytes(this, inbound) != FAILED)
+       {       /* there was traffic since last update or the KI does not support it */
+               update_usetime(this, inbound);
+       }
+       if (time)
+       {
+               *time = inbound ? this->my_usetime : this->other_usetime;
+       }
+       if (bytes)
+       {
+               *bytes = inbound ? this->my_usebytes : this->other_usebytes;
+       }
 }
 
 /**
@@ -869,8 +874,7 @@ child_sa_t * child_sa_create(host_t *me, host_t* other,
        this->public.get_proposal = (proposal_t*(*)(child_sa_t*))get_proposal;
        this->public.set_proposal = (void(*)(child_sa_t*, proposal_t *proposal))set_proposal;
        this->public.get_lifetime = (u_int32_t(*)(child_sa_t*, bool))get_lifetime;
-       this->public.get_usetime = (u_int32_t(*)(child_sa_t*, bool))get_usetime;
-       this->public.get_usebytes = (u_int64_t(*)(child_sa_t*, bool))get_usebytes;
+       this->public.get_usestats = (void(*)(child_sa_t*,bool,time_t*,u_int64_t*))get_usestats;
        this->public.has_encap = (bool(*)(child_sa_t*))has_encap;
        this->public.get_ipcomp = (ipcomp_transform_t(*)(child_sa_t*))get_ipcomp;
        this->public.set_ipcomp = (void(*)(child_sa_t*,ipcomp_transform_t))set_ipcomp;

diff --git a/src/charon/sa/child_sa.h b/src/charon/sa/child_sa.h
index 4f7f022..698da8b 100644 (file)
--- a/src/charon/sa/child_sa.h
+++ b/src/charon/sa/child_sa.h
@@ -85,11 +85,11 @@ extern enum_name_t *child_sa_state_names;
 
 /**
  * Represents an IPsec SAs between two hosts.
- * 
+ *
  * A child_sa_t contains two SAs. SAs for both
  * directions are managed in one child_sa_t object. Both
  * SAs and the policies have the same reqid.
- * 
+ *
  * The procedure for child sa setup is as follows:
  * - A gets SPIs for a all protocols in its proposals via child_sa_t.alloc
  * - A send the proposals with the allocated SPIs to B
@@ -98,7 +98,7 @@ extern enum_name_t *child_sa_state_names;
  * - B calls child_sa_t.install for both, the allocated and received SPI
  * - B sends the proposal with the allocated SPI to A
  * - A calls child_sa_t.install for both, the allocated and recevied SPI
- * 
+ *
  * Once SAs are set up, policies can be added using add_policies.
  */
 struct child_sa_t {
@@ -112,7 +112,7 @@ struct child_sa_t {
        
        /**
         * Get the reqid of the CHILD SA.
-        * 
+        *
         * Every CHILD_SA has a reqid. The kernel uses this ID to
         * identify it.
         *
@@ -131,19 +131,19 @@ struct child_sa_t {
         * Get the state of the CHILD_SA.
         *
         * @return                      CHILD_SA state
-        */     
+        */
        child_sa_state_t (*get_state) (child_sa_t *this);
        
        /**
         * Set the state of the CHILD_SA.
         *
         * @param state         state to set on CHILD_SA
-        */     
+        */
        void (*set_state) (child_sa_t *this, child_sa_state_t state);
        
        /**
         * Get the SPI of this CHILD_SA.
-        * 
+        *
         * Set the boolean parameter inbound to TRUE to
         * get the SPI for which we receive packets, use
         * FALSE to get those we use for sending packets.
@@ -155,7 +155,7 @@ struct child_sa_t {
        
        /**
         * Get the CPI of this CHILD_SA.
-        * 
+        *
         * Set the boolean parameter inbound to TRUE to
         * get the CPI for which we receive packets, use
         * FALSE to get those we use for sending packets.
@@ -202,7 +202,7 @@ struct child_sa_t {
        
        /**
         * Set the IPComp algorithm to use.
-        * 
+        *
         * @param ipcomp        the IPComp transform to use
         */
        void (*set_ipcomp)(child_sa_t *this, ipcomp_transform_t ipcomp);
@@ -219,7 +219,7 @@ struct child_sa_t {
         *
         * @param proposal      selected proposal
         */
-       void (*set_proposal)(child_sa_t *this, proposal_t *proposal);   
+       void (*set_proposal)(child_sa_t *this, proposal_t *proposal);
        
        /**
         * Check if this CHILD_SA uses UDP encapsulation.
@@ -237,27 +237,21 @@ struct child_sa_t {
        u_int32_t (*get_lifetime)(child_sa_t *this, bool hard);
        
        /**
-        * Get last use time of the CHILD_SA.
-        *
-        * @param inbound       TRUE for inbound traffic, FALSE for outbound
-        * @return                      time of last use in seconds
-        */
-       u_int32_t (*get_usetime)(child_sa_t *this, bool inbound);
-       
-       /**
-        * Get number of bytes processed by CHILD_SA.
+        * Get last use time and the number of bytes processed.
         *
         * @param inbound               TRUE for inbound traffic, FALSE for outbound
-        * @return                              number of processed bytes
+        * @param[out] time             time of last use in seconds (NULL to ignore)
+        * @param[out] bytes    number of processed bytes (NULL to ignore)
         */
-       u_int64_t (*get_usebytes)(child_sa_t *this, bool inbound);
-
+       void (*get_usestats)(child_sa_t *this, bool inbound, time_t *time,
+                                                u_int64_t *bytes);
+       
        /**
         * Get the traffic selectors list added for one side.
         *
         * @param local         TRUE for own traffic selectors, FALSE for remote
         * @return                      list of traffic selectors
-        */     
+        */
        linked_list_t* (*get_traffic_selectors) (child_sa_t *this, bool local);
        
        /**
@@ -304,7 +298,7 @@ struct child_sa_t {
         * @param my_ts         traffic selectors for local site
         * @param other_ts      traffic selectors for remote site
         * @return                      SUCCESS or FAILED
-        */     
+        */
        status_t (*add_policies)(child_sa_t *this, linked_list_t *my_ts_list,
                                                         linked_list_t *other_ts_list);
        /**

diff --git a/src/charon/sa/ike_sa.c b/src/charon/sa/ike_sa.c
index 1c2de17..be973a2 100644 (file)
--- a/src/charon/sa/ike_sa.c
+++ b/src/charon/sa/ike_sa.c
@@ -260,7 +260,7 @@ static time_t get_use_time(private_ike_sa_t* this, bool inbound)
 {
        enumerator_t *enumerator;
        child_sa_t *child_sa;
-       time_t use_time;
+       time_t use_time, current;
        
        if (inbound)
        {
@@ -273,7 +273,8 @@ static time_t get_use_time(private_ike_sa_t* this, bool inbound)
        enumerator = this->child_sas->create_enumerator(this->child_sas);
        while (enumerator->enumerate(enumerator, &child_sa))
        {
-               use_time = max(use_time, child_sa->get_usetime(child_sa, inbound));
+               child_sa->get_usestats(child_sa, inbound, &current, NULL);
+               use_time = max(use_time, current);
        }
        enumerator->destroy(enumerator);
        

diff --git a/src/charon/sa/tasks/child_delete.c b/src/charon/sa/tasks/child_delete.c
index 6e87ad9..af3f276 100644 (file)
--- a/src/charon/sa/tasks/child_delete.c
+++ b/src/charon/sa/tasks/child_delete.c
@@ -240,17 +240,19 @@ static void log_children(private_child_delete_t *this)
 {
        iterator_t *iterator;
        child_sa_t *child_sa;
+       u_int64_t bytes_in, bytes_out;
        
        iterator = this->child_sas->create_iterator(this->child_sas, TRUE);
        while (iterator->iterate(iterator, (void**)&child_sa))
        {
+               child_sa->get_usestats(child_sa, TRUE, NULL, &bytes_in);
+               child_sa->get_usestats(child_sa, FALSE, NULL, &bytes_out);
+               
                DBG0(DBG_IKE, "closing CHILD_SA %s{%d} "
                         "with SPIs %.8x_i (%lu bytes) %.8x_o (%lu bytes) and TS %#R=== %#R",
                         child_sa->get_name(child_sa), child_sa->get_reqid(child_sa),
-                        ntohl(child_sa->get_spi(child_sa, TRUE)),
-                        child_sa->get_usebytes(child_sa, TRUE),
-                        ntohl(child_sa->get_spi(child_sa, FALSE)),
-                        child_sa->get_usebytes(child_sa, FALSE),
+                        ntohl(child_sa->get_spi(child_sa, TRUE)), bytes_in,
+                        ntohl(child_sa->get_spi(child_sa, FALSE)), bytes_out,
                         child_sa->get_traffic_selectors(child_sa, TRUE),
                         child_sa->get_traffic_selectors(child_sa, FALSE));
        }