Re: len & can_dlc checks in tx path

[Wolfgang Grandegger]

Hi Oliver,

Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
> 
>>>> Please also check for useless "dlc > 8" checks in the
>>>> start_xmit function.
>>> I thought about that and i'm not really sure about it:
>>> What if anyone uses PF_PACKET with CAN netdevices, which is a possible 
>>> use-case?
>> OK.
> 
> Any ideas how to proceed in the start_xmit functions?
> 
> We could add something like this consistently to *all* start_xmit functions:
> 
> Index: drivers/net/can/sja1000/sja1000.c
> ===================================================================
> --- drivers/net/can/sja1000/sja1000.c (Revision 1095)
> +++ drivers/net/can/sja1000/sja1000.c (Arbeitskopie)
> @@ -257,6 +257,13 @@
>       uint8_t dreg;
>       int i;
> 
> +     if (WARN_ONCE(skb->len != sizeof(*cf) || cf->can_dlc > 8,
> +                   "Dropped non conform skbuf: len %d, can_dlc %d\n",
> +                   skb->len, cf->can_dlc)) {
> +             kfree_skb(skb);
> +             return 0;
> +     }
> +
>       netif_stop_queue(dev);
> 
>       fi = dlc = cf->can_dlc;

I'm just preparing a patch for the MSCAN touching the same topic, which
I have attached below. What do you think? I also wonder if we should
use dev_kfree_skb()?

Wolfgang.


--------------------------------------------------------------------
[PATCH] can: mscan: fix improper return if dlc < 8 in start_xmit function

The start_xmit function of the MSCAN Driver did return improperly if
the CAN dlc check failed (skb not freed and invalid return code). This
patch adds a proper check of the frame length and data size and returns
now correctly. Furthermore, a little typo has been fixed.
---
 drivers/net/can/mscan/mscan.c |   11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/net/can/mscan/mscan.c
===================================================================
--- linux-2.6.orig/drivers/net/can/mscan/mscan.c        2009-12-11 
19:52:05.000000000 +0100
+++ linux-2.6/drivers/net/can/mscan/mscan.c     2009-12-11 19:52:16.356553712 
+0100
@@ -4,7 +4,7 @@
  * Copyright (C) 2005-2006 Andrey Volkov <avol...@varma-el.com>,
  *                         Varma Electronics Oy
  * Copyright (C) 2008-2009 Wolfgang Grandegger <w...@grandegger.com>
- * Copytight (C) 2008-2009 Pengutronix <ker...@pengutronix.de>
+ * Copyright (C) 2008-2009 Pengutronix <ker...@pengutronix.de>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the version 2 of the GNU General Public License
@@ -177,8 +177,13 @@
        int i, rtr, buf_id;
        u32 can_id;
 
-       if (frame->can_dlc > 8)
-               return -EINVAL;
+       if (skb->len != sizeof(*frame) || frame->can_dlc > 8) {
+               dev_err(dev->dev.parent,
+                       "Dropping non-conform paket: len %d, can_dlc %d\n",
+                       skb->len, frame->can_dlc);
+               kfree_skb(skb);
+               return  NETDEV_TX_OK;
+       }
 
        out_8(&regs->cantier, 0);
 


_______________________________________________
Socketcan-core mailing list
Socketcan-core@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/socketcan-core