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

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to