All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Hartkopp <oliver@hartkopp.net>
To: David Miller <davem@davemloft.net>
Cc: Urs Thuermann <urs@isnogud.escape.de>,
	Luotao Fu <l.fu@pengutronix.de>,
	Michael Olbrich <m.olbrich@pengutronix.de>,
	Linux Netdev List <netdev@vger.kernel.org>
Subject: [PATCH net-2.6] can: Use WARN_ONCE() instead of BUG_ON() for sanity check in receive path
Date: Mon, 10 Aug 2009 13:27:09 +0200	[thread overview]
Message-ID: <4A80040D.3030009@hartkopp.net> (raw)

[-- Attachment #1: Type: text/plain, Size: 690 bytes --]

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).

Additionally it changes the return values to the common NET_RX_xxx constants.

Signed-off-by: Oliver Hartkopp <oliver@hartkopp.net>
Signed-off-by: Urs Thuermann <urs@isnogud.escape.de>
CC: Michael Olbrich <m.olbrich@pengutronix.de>
CC: Luotao Fu <l.fu@pengutronix.de>

---


[-- Attachment #2: af_can_convert_bug_to_warn.patch --]
[-- Type: text/x-patch, Size: 1050 bytes --]

diff --git a/net/can/af_can.c b/net/can/af_can.c
index e733725..ef1c43a 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -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++;
@@ -682,7 +686,11 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
 		can_stats.matches_delta++;
 	}
 
-	return 0;
+	return NET_RX_SUCCESS;
+
+drop:
+	kfree_skb(skb);
+	return NET_RX_DROP;
 }
 
 /*

             reply	other threads:[~2009-08-10 11:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-10 11:27 Oliver Hartkopp [this message]
2009-08-13  5:01 ` [PATCH net-2.6] can: Use WARN_ONCE() instead of BUG_ON() for sanity check in receive path David Miller
2009-08-14  5:57   ` Oliver Hartkopp
2009-08-14  7:13     ` David Miller
2009-08-14  8:11       ` Oliver Hartkopp

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A80040D.3030009@hartkopp.net \
    --to=oliver@hartkopp.net \
    --cc=davem@davemloft.net \
    --cc=l.fu@pengutronix.de \
    --cc=m.olbrich@pengutronix.de \
    --cc=netdev@vger.kernel.org \
    --cc=urs@isnogud.escape.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.