Re: [B.A.T.M.A.N.] [PATCH v2 2/2] batman-adv: Avoid crashing kernel with multicast asserts

["Linus Lüssing" <linus.luessing@c0d3.blue>]

Sounds good, that'll probably spare us some discussions about
BUG_ON()s on netdev :).

Acked-by: Linus Lüssing <linus.luessing@c0d3.blue>


On Fri, Jul 17, 2015 at 10:03:43AM +0200, Sven Eckelmann wrote:
> The BUG_ON added to the multicast code in commit 7f220ed1f063 ("batman-adv:
> Fix potential synchronization issues in mcast tvlv handler") will crash the
> kernel when the statement is true. This is not strictly required and a
> WARN_ON is enough to raise attention.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v2: Change commit message to new git commit description style
> 
>  net/batman-adv/multicast.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
> index 25ed931..d71c70e 100644
> --- a/net/batman-adv/multicast.c
> +++ b/net/batman-adv/multicast.c
> @@ -618,7 +618,7 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(!hlist_unhashed(node));
> +		WARN_ON(!hlist_unhashed(node));
>  
>  		hlist_add_head_rcu(node, head);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> @@ -629,7 +629,7 @@ static void batadv_mcast_want_unsnoop_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(hlist_unhashed(node));
> +		WARN_ON(hlist_unhashed(node));
>  
>  		hlist_del_init_rcu(node);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> @@ -663,7 +663,7 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(!hlist_unhashed(node));
> +		WARN_ON(!hlist_unhashed(node));
>  
>  		hlist_add_head_rcu(node, head);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> @@ -674,7 +674,7 @@ static void batadv_mcast_want_ipv4_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(hlist_unhashed(node));
> +		WARN_ON(hlist_unhashed(node));
>  
>  		hlist_del_init_rcu(node);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> @@ -708,7 +708,7 @@ static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(!hlist_unhashed(node));
> +		WARN_ON(!hlist_unhashed(node));
>  
>  		hlist_add_head_rcu(node, head);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> @@ -719,7 +719,7 @@ static void batadv_mcast_want_ipv6_update(struct batadv_priv *bat_priv,
>  
>  		spin_lock_bh(&bat_priv->mcast.want_lists_lock);
>  		/* flag checks above + mcast_handler_lock prevents this */
> -		BUG_ON(hlist_unhashed(node));
> +		WARN_ON(hlist_unhashed(node));
>  
>  		hlist_del_init_rcu(node);
>  		spin_unlock_bh(&bat_priv->mcast.want_lists_lock);
> -- 
> 2.1.4
>