Hi Sven
Thanks for the patches. I will test them out of Monday and let you
know if it fixes the crash.
Andrew
On Sat, Mar 05, 2016 at 04:09:16PM +0100, Sven Eckelmann wrote:
> The batadv_hardif_list list is checked in many situations and the items
> in this list are given to specialized functions to modify the routing
> behavior. At the moment each of these called functions has to check
> itself whether the received batadv_hard_iface has a refcount > 0 before
> it can increase the reference counter and use it in other objects.
>
> This can easily lead to problems because it is not easily visible where
> all callers of a function got the batadv_hard_iface object from and
> whether they already hold a valid reference.
>
> Checking the reference counter directly before calling a subfunction
> with a pointer from the batadv_hardif_list avoids this problem.
>
> Signed-off-by: Sven Eckelmann <[email protected]>
> ---
> net/batman-adv/bat_iv_ogm.c | 11 +++++++++++
> net/batman-adv/bat_v_ogm.c | 14 +++++++++++++-
> net/batman-adv/originator.c | 5 +++++
> net/batman-adv/send.c | 6 ++++++
> 4 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
> index 2c65668..b390ff9 100644
> --- a/net/batman-adv/bat_iv_ogm.c
> +++ b/net/batman-adv/bat_iv_ogm.c
> @@ -985,9 +985,15 @@ static void batadv_iv_ogm_schedule(struct
> batadv_hard_iface *hard_iface)
> list_for_each_entry_rcu(tmp_hard_iface, &batadv_hardif_list, list) {
> if (tmp_hard_iface->soft_iface != hard_iface->soft_iface)
> continue;
> +
> + if (!kref_get_unless_zero(&tmp_hard_iface->refcount))
> + continue;
> +
> batadv_iv_ogm_queue_add(bat_priv, *ogm_buff,
> *ogm_buff_len, hard_iface,
> tmp_hard_iface, 1, send_time);
> +
> + batadv_hardif_put(tmp_hard_iface);
> }
> rcu_read_unlock();
>
> @@ -1767,8 +1773,13 @@ static void batadv_iv_ogm_process(const struct sk_buff
> *skb, int ogm_offset,
> if (hard_iface->soft_iface != bat_priv->soft_iface)
> continue;
>
> + if (!kref_get_unless_zero(&hard_iface->refcount))
> + continue;
> +
> batadv_iv_ogm_process_per_outif(skb, ogm_offset, orig_node,
> if_incoming, hard_iface);
> +
> + batadv_hardif_put(hard_iface);
> }
> rcu_read_unlock();
>
> diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
> index 4155fa5..473ebb9 100644
> --- a/net/batman-adv/bat_v_ogm.c
> +++ b/net/batman-adv/bat_v_ogm.c
> @@ -26,6 +26,7 @@
> #include <linux/if_ether.h>
> #include <linux/jiffies.h>
> #include <linux/kernel.h>
> +#include <linux/kref.h>
> #include <linux/list.h>
> #include <linux/netdevice.h>
> #include <linux/random.h>
> @@ -176,6 +177,9 @@ static void batadv_v_ogm_send(struct work_struct *work)
> if (hard_iface->soft_iface != bat_priv->soft_iface)
> continue;
>
> + if (!kref_get_unless_zero(&hard_iface->refcount))
> + continue;
> +
> batadv_dbg(BATADV_DBG_BATMAN, bat_priv,
> "Sending own OGM2 packet (originator %pM, seqno %u,
> throughput %u, TTL %d) on interface %s [%pM]\n",
> ogm_packet->orig, ntohl(ogm_packet->seqno),
> @@ -185,10 +189,13 @@ static void batadv_v_ogm_send(struct work_struct *work)
>
> /* this skb gets consumed by batadv_v_ogm_send_to_if() */
> skb_tmp = skb_clone(skb, GFP_ATOMIC);
> - if (!skb_tmp)
> + if (!skb_tmp) {
> + batadv_hardif_put(hard_iface);
> break;
> + }
>
> batadv_v_ogm_send_to_if(skb_tmp, hard_iface);
> + batadv_hardif_put(hard_iface);
> }
> rcu_read_unlock();
>
> @@ -704,9 +711,14 @@ static void batadv_v_ogm_process(const struct sk_buff
> *skb, int ogm_offset,
> if (hard_iface->soft_iface != bat_priv->soft_iface)
> continue;
>
> + if (!kref_get_unless_zero(&hard_iface->refcount))
> + continue;
> +
> batadv_v_ogm_process_per_outif(bat_priv, ethhdr, ogm_packet,
> orig_node, neigh_node,
> if_incoming, hard_iface);
> +
> + batadv_hardif_put(hard_iface);
> }
> rcu_read_unlock();
> out:
> diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
> index e63d6a5..44c508a 100644
> --- a/net/batman-adv/originator.c
> +++ b/net/batman-adv/originator.c
> @@ -1165,6 +1165,9 @@ static bool batadv_purge_orig_node(struct batadv_priv
> *bat_priv,
> if (hard_iface->soft_iface != bat_priv->soft_iface)
> continue;
>
> + if (!kref_get_unless_zero(&hard_iface->refcount))
> + continue;
> +
> best_neigh_node = batadv_find_best_neighbor(bat_priv,
> orig_node,
> hard_iface);
> @@ -1172,6 +1175,8 @@ static bool batadv_purge_orig_node(struct batadv_priv
> *bat_priv,
> best_neigh_node);
> if (best_neigh_node)
> batadv_neigh_node_put(best_neigh_node);
> +
> + batadv_hardif_put(hard_iface);
> }
> rcu_read_unlock();
>
> diff --git a/net/batman-adv/send.c b/net/batman-adv/send.c
> index 3ce06e0..43a950e 100644
> --- a/net/batman-adv/send.c
> +++ b/net/batman-adv/send.c
> @@ -26,6 +26,7 @@
> #include <linux/if.h>
> #include <linux/jiffies.h>
> #include <linux/kernel.h>
> +#include <linux/kref.h>
> #include <linux/list.h>
> #include <linux/netdevice.h>
> #include <linux/printk.h>
> @@ -577,10 +578,15 @@ static void batadv_send_outstanding_bcast_packet(struct
> work_struct *work)
> if (forw_packet->num_packets >= hard_iface->num_bcasts)
> continue;
>
> + if (!kref_get_unless_zero(&hard_iface->refcount))
> + continue;
> +
> /* send a copy of the saved skb */
> skb1 = skb_clone(forw_packet->skb, GFP_ATOMIC);
> if (skb1)
> batadv_send_broadcast_skb(skb1, hard_iface);
> +
> + batadv_hardif_put(hard_iface);
> }
> rcu_read_unlock();
>
> --
> 2.7.0
>
