On 06.12.2010 14:06, Michal Sojka wrote:
> skb->sk is used in dev_pick_tx() which is called from dev_queue_xmit(). If
> sk points to an arbitrary magic value, dev_pick_tx() returns a wrong value,
> which can lead to various memory corruption bugs.

Dear Michal,

while searching for an alternative to ensure the single hop routing of CAN
skbs, i tried to use existing skb-fields that can detect this case, e.g.:

       if (skb->dev->ifindex != skb->skb_iif)
                return;

Finally i discussed with a colleague about common use-cases for CAN routings
and we wondered, if it would be ok to remove the one-hop restriction entirely,
like you already did here:

http://rtime.felk.cvut.cz/gitweb/shark/linux.git/commitdiff/92487e4f349cd7518cc3202662f42fea7d42ba73

The question is if we could allow

- the (currently enabled) routing to the originating interface (can0 -> can0)
- the routing over several hops (can0 -> vcan0 -> vcan1 -> can0)

without any restrictions.

As only root can create CAN routing entries with netlink-API we problably can
assume, that "he knows what he's doing" ... maybe accompanied by some warnings
from the cangw tool, when obviously strange configurations are to be written. 

What do you think about this removal of the one-hop restriction?

Regards,
Oliver


Index: gw.c
===================================================================
--- gw.c        (Revision 1223)
+++ gw.c        (Arbeitskopie)
@@ -58,7 +58,6 @@
 #include <socketcan/can/gw.h>
 #include <net/rtnetlink.h>
 #include <net/net_namespace.h>
-#include <net/sock.h>
 
 #include <socketcan/can/version.h> /* for RCSID. Removed by mkpatch script */
 RCSID("$Id$");
@@ -77,8 +76,6 @@
 
 static struct kmem_cache *cgw_cache __read_mostly;
 
-static struct sock gw_dummy_sk;
-
 /* structure that contains the (on-the-fly) CAN frame modifications */
 struct cf_mod {
        struct {
@@ -346,10 +343,6 @@
        struct sk_buff *nskb;
        int modidx = 0;
 
-       /* do not handle already routed frames */
-       if (skb->sk == &gw_dummy_sk)
-               return;
-
        if (!(gwj->dst.dev->flags & IFF_UP)) {
                gwj->dropped_frames++;
                return;
@@ -371,8 +364,7 @@
                return;
        }
 
-       /* mark routed frames with a 'special' sk value */
-       nskb->sk = &gw_dummy_sk;
+       /* set outgoing interface */
        nskb->dev = gwj->dst.dev;
 
        /* pointer to modifiable CAN frame */
@@ -929,11 +921,6 @@
        notifier.notifier_call = cgw_notifier;
        register_netdevice_notifier(&notifier);
 
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,33)
-       /* initialize struct for dev_pick_tx() */
-       sk_tx_queue_clear(&gw_dummy_sk);
-#endif
-
        if (__rtnl_register(PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs)) {
                unregister_netdevice_notifier(&notifier);
                kmem_cache_destroy(cgw_cache);
_______________________________________________
Socketcan-users mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-users

Reply via email to