diff mbox

[2/9] batman-adv: Check hard_iface refcnt when receiving skb

Message ID 1457190564-11419-2-git-send-email-sven@narfation.org (mailing list archive)
State Accepted, archived
Delegated to: Marek Lindner
Headers

Commit Message

Sven Eckelmann March 5, 2016, 3:09 p.m. UTC 
  The receive function may start processing an incoming packet while the
hard_iface is shut down in a different context. All called functions called
with the batadv_hard_iface object belonging to the incoming interface would
have to check whether the reference counter is still > 0.

This is rather error-prone because this check can be forgotten easily.
Instead check the reference counter when receiving the object to make sure
that all called functions have a valid reference.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 net/batman-adv/main.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Marek Lindner March 28, 2016, 2:27 p.m. UTC | #1 
On Saturday, March 05, 2016 16:09:17 Sven Eckelmann wrote:
> The receive function may start processing an incoming packet while the
> hard_iface is shut down in a different context. All called functions called
> with the batadv_hard_iface object belonging to the incoming interface would
> have to check whether the reference counter is still > 0.
> 
> This is rather error-prone because this check can be forgotten easily.
> Instead check the reference counter when receiving the object to make sure
> that all called functions have a valid reference.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
>  net/batman-adv/main.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

Applied in revision fee73a8.

Thanks,
Marek
diff mbox

Patch

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index d64ddb9..bda5f13 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -401,11 +401,19 @@  int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 
 	hard_iface = container_of(ptype, struct batadv_hard_iface,
 				  batman_adv_ptype);
+
+	/* don't receive packet for interface which gets shut down by batman-adv
+	 * or otherwise it may gets tried to be referenced again in other
+	 * structures like batadv_forw_packet
+	 */
+	if (!kref_get_unless_zero(&hard_iface->refcount))
+		goto err_out;
+
 	skb = skb_share_check(skb, GFP_ATOMIC);
 
 	/* skb was released by skb_share_check() */
 	if (!skb)
-		goto err_out;
+		goto err_put;
 
 	/* packet should hold at least type and version */
 	if (unlikely(!pskb_may_pull(skb, 2)))
@@ -448,6 +456,8 @@  int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	if (ret == NET_RX_DROP)
 		kfree_skb(skb);
 
+	batadv_hardif_put(hard_iface);
+
 	/* return NET_RX_SUCCESS in any case as we
 	 * most probably dropped the packet for
 	 * routing-logical reasons.
@@ -456,6 +466,8 @@  int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 
 err_free:
 	kfree_skb(skb);
+err_put:
+	batadv_hardif_put(hard_iface);
 err_out:
 	return NET_RX_DROP;
 }