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
signature.asc
Description: Digital signature
