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,