From patchwork Mon Jan 11 14:59:21 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Oliver Hartkopp X-Patchwork-Id: 42623 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 19C85B7C27 for ; Tue, 12 Jan 2010 01:59:46 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753242Ab0AKO7k (ORCPT ); Mon, 11 Jan 2010 09:59:40 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753153Ab0AKO7j (ORCPT ); Mon, 11 Jan 2010 09:59:39 -0500 Received: from mo-p00-ob.rzone.de ([81.169.146.160]:58183 "EHLO mo-p00-ob.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753139Ab0AKO7j (ORCPT ); Mon, 11 Jan 2010 09:59:39 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; t=1263221976; l=5864; s=domk; d=hartkopp.net; h=Content-Transfer-Encoding:Content-Type:Subject:CC:To:MIME-Version: From:Date:X-RZG-CLASS-ID:X-RZG-AUTH; bh=08tBQWyiguVgBdf63c/wuU7gxbk=; b=SCU8DZBdTch3KiJIzM5nY+a97WA72VwYhUNV2tD6Go/48dqISS6PnUI2SO4BQ6/sl2P MWODMNoky70cYg66v5Qwp5fb/IaHuN35u82KvMTRXJFVr1YdPrIIpIzx0grAPIe+2l0ar ayAYb+5SZwmlOm2pknvkfBt/sZP3JDt8Zfc= X-RZG-AUTH: :P2MHfkW8eP4Mre39l357AZT/I7AY/7nT2yrT1q0ngWNsKR9Dbc/nsazIMw6pTR6jn7s= X-RZG-CLASS-ID: mo00 Received: from [10.1.0.63] (81-188-95-90.xdsl.easynet.be [81.188.95.90]) by post.strato.de (fruni mo14) (RZmta 22.6) with ESMTP id d00f77m0BEx4z1 ; Mon, 11 Jan 2010 15:59:27 +0100 (MET) Message-ID: <4B4B3CC9.7040801@hartkopp.net> Date: Mon, 11 Jan 2010 15:59:21 +0100 From: Oliver Hartkopp User-Agent: Mozilla-Thunderbird 2.0.0.22 (X11/20090706) MIME-Version: 1.0 To: David Miller , Wolfgang Grandegger CC: Linux Netdev List , SocketCAN Core Mailing List Subject: [PATCH net-next-2.6 v2] can: Unify droping of invalid tx skbs and netdev stats Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org To prevent the CAN drivers to operate on invalid socketbuffers the skbs are now checked and silently dropped at the xmit-function consistently. Also the netdev stats are consistently using the CAN data length code (dlc) for [rx|tx]_bytes now. Signed-off-by: Oliver Hartkopp --- -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c index 166cc7e..70bff87 100644 --- a/drivers/net/can/at91_can.c +++ b/drivers/net/can/at91_can.c @@ -342,6 +342,12 @@ static netdev_tx_t at91_start_xmit(struct sk_buff *skb, struct net_device *dev) unsigned int mb, prio; u32 reg_mid, reg_mcr; + if (invalid_can_skb(skb)) { + kfree_skb(skb); + dev->stats.tx_dropped++; + return NETDEV_TX_OK; + } + mb = get_tx_next_mb(priv); prio = get_tx_next_prio(priv); diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c index 0ec1524..b0bf359 100644 --- a/drivers/net/can/bfin_can.c +++ b/drivers/net/can/bfin_can.c @@ -318,6 +318,12 @@ static int bfin_can_start_xmit(struct sk_buff *skb, struct net_device *dev) u16 val; int i; + if (invalid_can_skb(skb)) { + kfree_skb(skb); + dev->stats.tx_dropped++; + return NETDEV_TX_OK; + } + netif_stop_queue(dev); /* fill id */ diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c index 9c5a153..4df3b40 100644 --- a/drivers/net/can/mcp251x.c +++ b/drivers/net/can/mcp251x.c @@ -494,9 +494,8 @@ static netdev_tx_t mcp251x_hard_start_xmit(struct sk_buff *skb, return NETDEV_TX_BUSY; } - if (skb->len != sizeof(struct can_frame)) { - dev_err(&spi->dev, "dropping packet - bad length\n"); - dev_kfree_skb(skb); + if (invalid_can_skb(skb)) { + kfree_skb(skb); net->stats.tx_dropped++; return NETDEV_TX_OK; } diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c index 542a4f7..3abad97 100644 --- a/drivers/net/can/sja1000/sja1000.c +++ b/drivers/net/can/sja1000/sja1000.c @@ -249,6 +249,12 @@ static netdev_tx_t sja1000_start_xmit(struct sk_buff *skb, uint8_t dreg; int i; + if (invalid_can_skb(skb)) { + kfree_skb(skb); + dev->stats.tx_dropped++; + return NETDEV_TX_OK; + } + netif_stop_queue(dev); fi = dlc = cf->can_dlc; diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c index 5c993c2..0ec9859 100644 --- a/drivers/net/can/ti_hecc.c +++ b/drivers/net/can/ti_hecc.c @@ -477,6 +477,12 @@ static netdev_tx_t ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev) u32 mbxno, mbx_mask, data; unsigned long flags; + if (invalid_can_skb(skb)) { + kfree_skb(skb); + ndev->stats.tx_dropped++; + return NETDEV_TX_OK; + } + mbxno = get_tx_head_mb(priv); mbx_mask = BIT(mbxno); spin_lock_irqsave(&priv->mbx_lock, flags); @@ -491,7 +497,6 @@ static netdev_tx_t ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev) spin_unlock_irqrestore(&priv->mbx_lock, flags); /* Prepare mailbox for transmission */ - data = min_t(u8, cf->can_dlc, 8); if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */ data |= HECC_CANMCF_RTR; data |= get_tx_head_prio(priv) << 8; diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c index efbb05c..a375781 100644 --- a/drivers/net/can/usb/ems_usb.c +++ b/drivers/net/can/usb/ems_usb.c @@ -767,6 +767,12 @@ static netdev_tx_t ems_usb_start_xmit(struct sk_buff *skb, struct net_device *ne size_t size = CPC_HEADER_SIZE + CPC_MSG_HEADER_LEN + sizeof(struct cpc_can_msg); + if (invalid_can_skb(skb)) { + kfree_skb(skb); + stats->tx_dropped++; + return NETDEV_TX_OK; + } + /* create a URB, and a buffer for it, and copy the data to the URB */ urb = usb_alloc_urb(0, GFP_ATOMIC); if (!urb) { diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c index 80ac563..66411d5 100644 --- a/drivers/net/can/vcan.c +++ b/drivers/net/can/vcan.c @@ -70,10 +70,11 @@ MODULE_PARM_DESC(echo, "Echo sent frames (for testing). Default: 0 (Off)"); static void vcan_rx(struct sk_buff *skb, struct net_device *dev) { + struct can_frame *cf = (struct can_frame *)skb->data; struct net_device_stats *stats = &dev->stats; stats->rx_packets++; - stats->rx_bytes += skb->len; + stats->rx_bytes += cf->can_dlc; skb->protocol = htons(ETH_P_CAN); skb->pkt_type = PACKET_BROADCAST; @@ -85,11 +86,18 @@ static void vcan_rx(struct sk_buff *skb, struct net_device *dev) static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev) { + struct can_frame *cf = (struct can_frame *)skb->data; struct net_device_stats *stats = &dev->stats; int loop; + if (unlikely(skb->len != sizeof(*cf) || cf->can_dlc > 8)) { + kfree_skb(skb); + stats->tx_dropped++; + return NETDEV_TX_OK; + } + stats->tx_packets++; - stats->tx_bytes += skb->len; + stats->tx_bytes += cf->can_dlc; /* set flag whether this packet has to be looped back */ loop = skb->pkt_type == PACKET_LOOPBACK; @@ -103,7 +111,7 @@ static netdev_tx_t vcan_tx(struct sk_buff *skb, struct net_device *dev) * CAN core already did the echo for us */ stats->rx_packets++; - stats->rx_bytes += skb->len; + stats->rx_bytes += cf->can_dlc; } kfree_skb(skb); return NETDEV_TX_OK; diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index 3db7767..e0703c3 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -60,6 +60,14 @@ struct can_priv { */ #define get_can_dlc(i) (min_t(__u8, (i), 8)) +/* check if a given socketbuffer does not contain a valid CAN frame */ +static inline int invalid_can_skb(struct sk_buff *skb) +{ + const struct can_frame *cf = (struct can_frame *)skb->data; + + return unlikely(skb->len != sizeof(*cf) || cf->can_dlc > 8); +} + struct net_device *alloc_candev(int sizeof_priv, unsigned int echo_skb_max); void free_candev(struct net_device *dev);