On Saturday, March 05, 2016 16:09:19 Sven Eckelmann wrote:
The callers of the functions using batadv_hard_iface objects have to make sure that they already hold a valid reference. The subfunctions don't have to check whether the reference counter is > 0 because this was already checked by the callers.
You say the callers have to make sure that valid references exist but do they or is this coming later or .. ?
--- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -679,12 +679,6 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, unsigned char *skb_buff; unsigned int skb_size;
- if (!kref_get_unless_zero(&if_incoming->refcount))
return;
- if (!kref_get_unless_zero(&if_outgoing->refcount))
goto out_free_incoming;
- /* own packet should always be scheduled */ if (!own_packet) { if (!batadv_atomic_dec_not_zero(&bat_priv->batman_queue_left)) {
@@ -716,6 +710,8 @@ static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff, forw_packet_aggr->packet_len = packet_len; memcpy(skb_buff, packet_buff, packet_len);
- kref_get(&if_incoming->refcount);
- kref_get(&if_outgoing->refcount); forw_packet_aggr->own = own_packet; forw_packet_aggr->if_incoming = if_incoming; forw_packet_aggr->if_outgoing = if_outgoing;
@@ -747,7 +743,6 @@ out_nomem: atomic_inc(&bat_priv->batman_queue_left); out_free_outgoing: batadv_hardif_put(if_outgoing); -out_free_incoming: batadv_hardif_put(if_incoming); }
This introduces a refcount imbalance If I am not mistaken. At the beginning of the function we jump to 'out_free_outgoing'.
Cheers, Marek