Skip to contentSkip to conversation

can: Use WARN_ONCE() instead of BUG_ON() for sanity check in receive path

PATCH Applied Oliver Hartkopp 2 participants 2 messages Aug 14, 2009

Subsystems (3)
Networking Network drivers CAN network layer
lore
Share & tools
lore

Oliver Hartkopp

author
#0 lore raw

PATCH 1/1 can: Use WARN_ONCE() instead of BUG_ON() for sanity check in receive path

To ensure a proper handling of CAN frames transported in skbuffs some checks need to be performed at receive time.

As stated by Michael Olbrich and Luotao Fu BUG_ON() might be to restrictive. This is right as we can just drop the non conform skbuff and the Kernel can continue working.

This patch replaces the BUG_ON() with a WARN_ONCE() so that the system remains healthy but we made the problem visible (once).

Signed-off-by Oliver Hartkopp <[email protected]>
Signed-off-by Urs Thuermann <[email protected]>
CC Michael Olbrich <[email protected]>
CC Luotao Fu <[email protected]>

---

net/can/af_can.c +13 -5
index e733725..f9c027b 100644
@@ -651,12 +651,16 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
 	struct can_frame *cf = (struct can_frame *)skb->data;
 	int matches;
 
-	if (dev->type != ARPHRD_CAN || !net_eq(dev_net(dev), &init_net)) {
-		kfree_skb(skb);
-		return 0;
-	}
+	if (!net_eq(dev_net(dev), &init_net))
+		goto drop;
 
-	BUG_ON(skb->len != sizeof(struct can_frame) || cf->can_dlc > 8);
+	if (WARN_ONCE(dev->type != ARPHRD_CAN ||
+		      skb->len != sizeof(struct can_frame) ||
+		      cf->can_dlc > 8,
+		      "PF_CAN: dropped non conform skbuf: "
+		      "dev type %d, len %d, can_dlc %d\n",
+		      dev->type, skb->len, cf->can_dlc))
+		goto drop;
 
 	/* update statistics */
 	can_stats.rx_frames++;

@@ -683,6 +687,10 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
 	}
 
 	return 0;
+
+drop:
+	kfree_skb(skb);
+	return 0;
 }
 
 /*
maintainer Applied
#1 lore raw

From: Oliver Hartkopp <[email protected]>
Date: Fri, 14 Aug 2009 10:54:25 +0200

Quoted 7 lines

To ensure a proper handling of CAN frames transported in skbuffs some checks need to be performed at receive time.

As stated by Michael Olbrich and Luotao Fu BUG_ON() might be to restrictive. This is right as we can just drop the non conform skbuff and the Kernel can continue working.

Quoted 5 lines

This patch replaces the BUG_ON() with a WARN_ONCE() so that the system remains healthy but we made the problem visible (once).

Signed-off-by Oliver Hartkopp <[email protected]>
Signed-off-by Urs Thuermann <[email protected]>

Applied, thanks.