Re: [B.A.T.M.A.N.] Use of ref_get_unless_zero() ?

[Antonio Quartulli <a@unstable.cc>]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]

On Fri, Mar 04, 2016 at 04:21:32PM +0100, Andrew Lunn wrote:
> Hi
> 
> I'm sometimes getting a crash after removing a hard interface when the
> batadv_send_outstanding_bat_org_packet() is called in a work queue.
> It calls
> 
> static void batadv_iv_ogm_aggregate_new(const unsigned char *packet_buff,
>                                         int packet_len, unsigned long send_time,
>                                         bool direct_link,
>                                         struct batadv_hard_iface *if_incoming,
>                                         struct batadv_hard_iface *if_outgoing,
>                                         int own_packet)
> {
>         struct batadv_priv *bat_priv = netdev_priv(if_incoming->soft_iface);
>         struct batadv_forw_packet *forw_packet_aggr;
>         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;
> 
> 
> Given that we have:
> 
> static inline void batadv_hardif_put(struct batadv_hard_iface *hard_iface)
> {
>         kref_put(&hard_iface->refcount, batadv_hardif_release);
> }
> 
> does using kref_get_unless_zero() make sense? If it is zero, hasn't it
> been freed by the kref_put that set it to zero?

Not sure if this is the case but what if batadv_iv_ogm_aggregate_new() is called
within a rcu_read protected context concurrent to the kref_put setting the
refcount to zero ?

If I am not wrong, in this case if_incoming/outgoing will
still be valid (until the rcu_read_unlock()) but the refcount will be 0.

Does it make sense ?

Cheers,


-- 
Antonio Quartulli

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]