net: introduce CAN specific pointer in the struct net_device
authorOleksij Rempel <o.rempel@pengutronix.de>
Tue, 23 Feb 2021 07:01:26 +0000 (08:01 +0100)
committerJakub Kicinski <kuba@kernel.org>
Wed, 24 Feb 2021 22:32:15 +0000 (14:32 -0800)
Since 20dd3850bcf8 ("can: Speed up CAN frame receiption by using
ml_priv") the CAN framework uses per device specific data in the AF_CAN
protocol. For this purpose the struct net_device->ml_priv is used. Later
the ml_priv usage in CAN was extended for other users, one of them being
CAN_J1939.

Later in the kernel ml_priv was converted to an union, used by other
drivers. E.g. the tun driver started storing it's stats pointer.

Since tun devices can claim to be a CAN device, CAN specific protocols
will wrongly interpret this pointer, which will cause system crashes.
Mostly this issue is visible in the CAN_J1939 stack.

To fix this issue, we request a dedicated CAN pointer within the
net_device struct.

Reported-by: syzbot+5138c4dd15a0401bec7b@syzkaller.appspotmail.com
Fixes: 20dd3850bcf8 ("can: Speed up CAN frame receiption by using ml_priv")
Fixes: ffd956eef69b ("can: introduce CAN midlayer private and allocate it automatically")
Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Fixes: 497a5757ce4e ("tun: switch to net core provided statistics counters")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/r/20210223070127.4538-1-o.rempel@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
drivers/net/can/dev/dev.c
drivers/net/can/slcan.c
drivers/net/can/vcan.c
drivers/net/can/vxcan.c
include/linux/can/can-ml.h
include/linux/netdevice.h
net/can/af_can.c
net/can/j1939/main.c
net/can/j1939/socket.c
net/can/proc.c

index d9281ae..311d856 100644 (file)
@@ -239,6 +239,7 @@ void can_setup(struct net_device *dev)
 struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
                                    unsigned int txqs, unsigned int rxqs)
 {
+       struct can_ml_priv *can_ml;
        struct net_device *dev;
        struct can_priv *priv;
        int size;
@@ -270,7 +271,8 @@ struct net_device *alloc_candev_mqs(int sizeof_priv, unsigned int echo_skb_max,
        priv = netdev_priv(dev);
        priv->dev = dev;
 
-       dev->ml_priv = (void *)priv + ALIGN(sizeof_priv, NETDEV_ALIGN);
+       can_ml = (void *)priv + ALIGN(sizeof_priv, NETDEV_ALIGN);
+       can_set_ml_priv(dev, can_ml);
 
        if (echo_skb_max) {
                priv->echo_skb_max = echo_skb_max;
index a1bd1be..30c8d53 100644 (file)
@@ -516,6 +516,7 @@ static struct slcan *slc_alloc(void)
        int i;
        char name[IFNAMSIZ];
        struct net_device *dev = NULL;
+       struct can_ml_priv *can_ml;
        struct slcan       *sl;
        int size;
 
@@ -538,7 +539,8 @@ static struct slcan *slc_alloc(void)
 
        dev->base_addr  = i;
        sl = netdev_priv(dev);
-       dev->ml_priv = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
+       can_ml = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
+       can_set_ml_priv(dev, can_ml);
 
        /* Initialize channel control data */
        sl->magic = SLCAN_MAGIC;
index 39ca14b..067705e 100644 (file)
@@ -153,7 +153,7 @@ static void vcan_setup(struct net_device *dev)
        dev->addr_len           = 0;
        dev->tx_queue_len       = 0;
        dev->flags              = IFF_NOARP;
-       dev->ml_priv            = netdev_priv(dev);
+       can_set_ml_priv(dev, netdev_priv(dev));
 
        /* set flags according to driver capabilities */
        if (echo)
index f9a524c..8861a7d 100644 (file)
@@ -141,6 +141,8 @@ static const struct net_device_ops vxcan_netdev_ops = {
 
 static void vxcan_setup(struct net_device *dev)
 {
+       struct can_ml_priv *can_ml;
+
        dev->type               = ARPHRD_CAN;
        dev->mtu                = CANFD_MTU;
        dev->hard_header_len    = 0;
@@ -149,7 +151,9 @@ static void vxcan_setup(struct net_device *dev)
        dev->flags              = (IFF_NOARP|IFF_ECHO);
        dev->netdev_ops         = &vxcan_netdev_ops;
        dev->needs_free_netdev  = true;
-       dev->ml_priv            = netdev_priv(dev) + ALIGN(sizeof(struct vxcan_priv), NETDEV_ALIGN);
+
+       can_ml = netdev_priv(dev) + ALIGN(sizeof(struct vxcan_priv), NETDEV_ALIGN);
+       can_set_ml_priv(dev, can_ml);
 }
 
 /* forward declaration for rtnl_create_link() */
index 2f5d731..8afa92d 100644 (file)
@@ -44,6 +44,7 @@
 
 #include <linux/can.h>
 #include <linux/list.h>
+#include <linux/netdevice.h>
 
 #define CAN_SFF_RCV_ARRAY_SZ (1 << CAN_SFF_ID_BITS)
 #define CAN_EFF_RCV_HASH_BITS 10
@@ -65,4 +66,15 @@ struct can_ml_priv {
 #endif
 };
 
+static inline struct can_ml_priv *can_get_ml_priv(struct net_device *dev)
+{
+       return netdev_get_ml_priv(dev, ML_PRIV_CAN);
+}
+
+static inline void can_set_ml_priv(struct net_device *dev,
+                                  struct can_ml_priv *ml_priv)
+{
+       netdev_set_ml_priv(dev, ml_priv, ML_PRIV_CAN);
+}
+
 #endif /* CAN_ML_H */
index ddf4cfc..f06fbee 100644 (file)
@@ -1584,6 +1584,12 @@ enum netdev_priv_flags {
 #define IFF_L3MDEV_RX_HANDLER          IFF_L3MDEV_RX_HANDLER
 #define IFF_LIVE_RENAME_OK             IFF_LIVE_RENAME_OK
 
+/* Specifies the type of the struct net_device::ml_priv pointer */
+enum netdev_ml_priv_type {
+       ML_PRIV_NONE,
+       ML_PRIV_CAN,
+};
+
 /**
  *     struct net_device - The DEVICE structure.
  *
@@ -1779,6 +1785,7 @@ enum netdev_priv_flags {
  *     @nd_net:                Network namespace this network device is inside
  *
  *     @ml_priv:       Mid-layer private
+ *     @ml_priv_type:  Mid-layer private type
  *     @lstats:        Loopback statistics
  *     @tstats:        Tunnel statistics
  *     @dstats:        Dummy statistics
@@ -2094,8 +2101,10 @@ struct net_device {
        possible_net_t                  nd_net;
 
        /* mid-layer private */
+       void                            *ml_priv;
+       enum netdev_ml_priv_type        ml_priv_type;
+
        union {
-               void                                    *ml_priv;
                struct pcpu_lstats __percpu             *lstats;
                struct pcpu_sw_netstats __percpu        *tstats;
                struct pcpu_dstats __percpu             *dstats;
@@ -2286,6 +2295,29 @@ static inline void netdev_reset_rx_headroom(struct net_device *dev)
        netdev_set_rx_headroom(dev, -1);
 }
 
+static inline void *netdev_get_ml_priv(struct net_device *dev,
+                                      enum netdev_ml_priv_type type)
+{
+       if (dev->ml_priv_type != type)
+               return NULL;
+
+       return dev->ml_priv;
+}
+
+static inline void netdev_set_ml_priv(struct net_device *dev,
+                                     void *ml_priv,
+                                     enum netdev_ml_priv_type type)
+{
+       WARN(dev->ml_priv_type && dev->ml_priv_type != type,
+            "Overwriting already set ml_priv_type (%u) with different ml_priv_type (%u)!\n",
+            dev->ml_priv_type, type);
+       WARN(!dev->ml_priv_type && dev->ml_priv,
+            "Overwriting already set ml_priv and ml_priv_type is ML_PRIV_NONE!\n");
+
+       dev->ml_priv = ml_priv;
+       dev->ml_priv_type = type;
+}
+
 /*
  * Net namespace inlines
  */
index 837bb8a..cce2af1 100644 (file)
@@ -304,8 +304,8 @@ static struct can_dev_rcv_lists *can_dev_rcv_lists_find(struct net *net,
                                                        struct net_device *dev)
 {
        if (dev) {
-               struct can_ml_priv *ml_priv = dev->ml_priv;
-               return &ml_priv->dev_rcv_lists;
+               struct can_ml_priv *can_ml = can_get_ml_priv(dev);
+               return &can_ml->dev_rcv_lists;
        } else {
                return net->can.rx_alldev_list;
        }
@@ -790,25 +790,6 @@ void can_proto_unregister(const struct can_proto *cp)
 }
 EXPORT_SYMBOL(can_proto_unregister);
 
-/* af_can notifier to create/remove CAN netdevice specific structs */
-static int can_notifier(struct notifier_block *nb, unsigned long msg,
-                       void *ptr)
-{
-       struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-
-       if (dev->type != ARPHRD_CAN)
-               return NOTIFY_DONE;
-
-       switch (msg) {
-       case NETDEV_REGISTER:
-               WARN(!dev->ml_priv,
-                    "No CAN mid layer private allocated, please fix your driver and use alloc_candev()!\n");
-               break;
-       }
-
-       return NOTIFY_DONE;
-}
-
 static int can_pernet_init(struct net *net)
 {
        spin_lock_init(&net->can.rcvlists_lock);
@@ -876,11 +857,6 @@ static const struct net_proto_family can_family_ops = {
        .owner  = THIS_MODULE,
 };
 
-/* notifier block for netdevice event */
-static struct notifier_block can_netdev_notifier __read_mostly = {
-       .notifier_call = can_notifier,
-};
-
 static struct pernet_operations can_pernet_ops __read_mostly = {
        .init = can_pernet_init,
        .exit = can_pernet_exit,
@@ -911,17 +887,12 @@ static __init int can_init(void)
        err = sock_register(&can_family_ops);
        if (err)
                goto out_sock;
-       err = register_netdevice_notifier(&can_netdev_notifier);
-       if (err)
-               goto out_notifier;
 
        dev_add_pack(&can_packet);
        dev_add_pack(&canfd_packet);
 
        return 0;
 
-out_notifier:
-       sock_unregister(PF_CAN);
 out_sock:
        unregister_pernet_subsys(&can_pernet_ops);
 out_pernet:
@@ -935,7 +906,6 @@ static __exit void can_exit(void)
        /* protocol unregister */
        dev_remove_pack(&canfd_packet);
        dev_remove_pack(&can_packet);
-       unregister_netdevice_notifier(&can_netdev_notifier);
        sock_unregister(PF_CAN);
 
        unregister_pernet_subsys(&can_pernet_ops);
index bb914d8..da3a7a7 100644 (file)
@@ -140,9 +140,9 @@ static struct j1939_priv *j1939_priv_create(struct net_device *ndev)
 static inline void j1939_priv_set(struct net_device *ndev,
                                  struct j1939_priv *priv)
 {
-       struct can_ml_priv *can_ml_priv = ndev->ml_priv;
+       struct can_ml_priv *can_ml = can_get_ml_priv(ndev);
 
-       can_ml_priv->j1939_priv = priv;
+       can_ml->j1939_priv = priv;
 }
 
 static void __j1939_priv_release(struct kref *kref)
@@ -211,12 +211,9 @@ static void __j1939_rx_release(struct kref *kref)
 /* get pointer to priv without increasing ref counter */
 static inline struct j1939_priv *j1939_ndev_to_priv(struct net_device *ndev)
 {
-       struct can_ml_priv *can_ml_priv = ndev->ml_priv;
+       struct can_ml_priv *can_ml = can_get_ml_priv(ndev);
 
-       if (!can_ml_priv)
-               return NULL;
-
-       return can_ml_priv->j1939_priv;
+       return can_ml->j1939_priv;
 }
 
 static struct j1939_priv *j1939_priv_get_by_ndev_locked(struct net_device *ndev)
@@ -225,9 +222,6 @@ static struct j1939_priv *j1939_priv_get_by_ndev_locked(struct net_device *ndev)
 
        lockdep_assert_held(&j1939_netdev_lock);
 
-       if (ndev->type != ARPHRD_CAN)
-               return NULL;
-
        priv = j1939_ndev_to_priv(ndev);
        if (priv)
                j1939_priv_get(priv);
@@ -348,15 +342,16 @@ static int j1939_netdev_notify(struct notifier_block *nb,
                               unsigned long msg, void *data)
 {
        struct net_device *ndev = netdev_notifier_info_to_dev(data);
+       struct can_ml_priv *can_ml = can_get_ml_priv(ndev);
        struct j1939_priv *priv;
 
+       if (!can_ml)
+               goto notify_done;
+
        priv = j1939_priv_get_by_ndev(ndev);
        if (!priv)
                goto notify_done;
 
-       if (ndev->type != ARPHRD_CAN)
-               goto notify_put;
-
        switch (msg) {
        case NETDEV_DOWN:
                j1939_cancel_active_session(priv, NULL);
@@ -365,7 +360,6 @@ static int j1939_netdev_notify(struct notifier_block *nb,
                break;
        }
 
-notify_put:
        j1939_priv_put(priv);
 
 notify_done:
index f239665..56aa661 100644 (file)
@@ -12,6 +12,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/can/can-ml.h>
 #include <linux/can/core.h>
 #include <linux/can/skb.h>
 #include <linux/errqueue.h>
@@ -453,6 +454,7 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
                j1939_jsk_del(priv, jsk);
                j1939_local_ecu_put(priv, jsk->addr.src_name, jsk->addr.sa);
        } else {
+               struct can_ml_priv *can_ml;
                struct net_device *ndev;
 
                ndev = dev_get_by_index(net, addr->can_ifindex);
@@ -461,15 +463,8 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
                        goto out_release_sock;
                }
 
-               if (ndev->type != ARPHRD_CAN) {
-                       dev_put(ndev);
-                       ret = -ENODEV;
-                       goto out_release_sock;
-               }
-
-               if (!ndev->ml_priv) {
-                       netdev_warn_once(ndev,
-                                        "No CAN mid layer private allocated, please fix your driver and use alloc_candev()!\n");
+               can_ml = can_get_ml_priv(ndev);
+               if (!can_ml) {
                        dev_put(ndev);
                        ret = -ENODEV;
                        goto out_release_sock;
index 5ea8695..b15760b 100644 (file)
@@ -322,8 +322,11 @@ static int can_rcvlist_proc_show(struct seq_file *m, void *v)
 
        /* receive list for registered CAN devices */
        for_each_netdev_rcu(net, dev) {
-               if (dev->type == ARPHRD_CAN && dev->ml_priv)
-                       can_rcvlist_proc_show_one(m, idx, dev, dev->ml_priv);
+               struct can_ml_priv *can_ml = can_get_ml_priv(dev);
+
+               if (can_ml)
+                       can_rcvlist_proc_show_one(m, idx, dev,
+                                                 &can_ml->dev_rcv_lists);
        }
 
        rcu_read_unlock();
@@ -375,8 +378,10 @@ static int can_rcvlist_sff_proc_show(struct seq_file *m, void *v)
 
        /* sff receive list for registered CAN devices */
        for_each_netdev_rcu(net, dev) {
-               if (dev->type == ARPHRD_CAN && dev->ml_priv) {
-                       dev_rcv_lists = dev->ml_priv;
+               struct can_ml_priv *can_ml = can_get_ml_priv(dev);
+
+               if (can_ml) {
+                       dev_rcv_lists = &can_ml->dev_rcv_lists;
                        can_rcvlist_proc_show_array(m, dev, dev_rcv_lists->rx_sff,
                                                    ARRAY_SIZE(dev_rcv_lists->rx_sff));
                }
@@ -406,8 +411,10 @@ static int can_rcvlist_eff_proc_show(struct seq_file *m, void *v)
 
        /* eff receive list for registered CAN devices */
        for_each_netdev_rcu(net, dev) {
-               if (dev->type == ARPHRD_CAN && dev->ml_priv) {
-                       dev_rcv_lists = dev->ml_priv;
+               struct can_ml_priv *can_ml = can_get_ml_priv(dev);
+
+               if (can_ml) {
+                       dev_rcv_lists = &can_ml->dev_rcv_lists;
                        can_rcvlist_proc_show_array(m, dev, dev_rcv_lists->rx_eff,
                                                    ARRAY_SIZE(dev_rcv_lists->rx_eff));
                }