On Friday 04 March 2016 16:21:32 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?
At least it makes sense for the outgoing interface because it is only in a
rcu_read_lock in batadv_iv_ogm_schedule (batadv_iv_ogm_queue_add ->
batadv_iv_ogm_aggregate_new). The batadv_hardif_list is traversed with
list_for_each_entry_rcu and it is expected that one entry (maybe) gets dropped
from the list. The batadv_hardif_release will only queue the actual free of
the memory (kfree_rcu) and every function which wants to get a reference has
to increase the counter with kref_get_unless_zero to check that it is not
actually in the waiting-to-be-freed-phase.
But you have something which needs to be fixed (you see a crash). Question is
what is causing the crash and what can be done against it. I am currently
wondering how the if_incoming interface is being protected. It is not fetched
from a list via a rcu list access primitive and it is not protected via
rcu_read_lock. I can also not see where the reference for the forw_packet-
>if_incoming is increased. It is just accessed in
batadv_send_outstanding_bat_ogm_packet (and later send to the mentioned
function via batadv_schedule_bat_ogm). Also batadv_add_bcast_packet_to_list
doesn't increase the reference counter for if_incoming before adding to the
forward packet. So I would just say that the reference counting for
batadv_hard_iface is broken.
Kind regards,
Sven
signature.asc
Description: This is a digitally signed message part.
