lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 15 Jan 2021 15:30:35 +0100
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     mkl@...gutronix.de, "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        Robin van der Gracht <robin@...tonic.nl>
Cc:     Oleksij Rempel <o.rempel@...gutronix.de>,
        syzbot+5138c4dd15a0401bec7b@...kaller.appspotmail.com,
        kernel@...gutronix.de, linux-can@...r.kernel.org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [RFC PATCH net 1/2] net: introduce CAN specific pointer in the struct net_device

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@...kaller.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@...gutronix.de>
---
 drivers/net/can/dev/dev.c |  2 +-
 drivers/net/can/slcan.c   |  2 +-
 drivers/net/can/vcan.c    |  2 +-
 drivers/net/can/vxcan.c   |  2 +-
 include/linux/netdevice.h |  4 ++++
 net/can/af_can.c          |  4 ++--
 net/can/j1939/main.c      |  4 ++--
 net/can/j1939/socket.c    |  2 +-
 net/can/proc.c            | 13 +++++++------
 9 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
index f580f0ac6497..0e5265cb635f 100644
--- a/drivers/net/can/dev/dev.c
+++ b/drivers/net/can/dev/dev.c
@@ -269,7 +269,7 @@ 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);
+	dev->can = (void *)priv + ALIGN(sizeof_priv, NETDEV_ALIGN);
 
 	if (echo_skb_max) {
 		priv->echo_skb_max = echo_skb_max;
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index a1bd1be09548..f703bd3b7415 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -538,7 +538,7 @@ static struct slcan *slc_alloc(void)
 
 	dev->base_addr  = i;
 	sl = netdev_priv(dev);
-	dev->ml_priv = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
+	dev->can = (void *)sl + ALIGN(sizeof(*sl), NETDEV_ALIGN);
 
 	/* Initialize channel control data */
 	sl->magic = SLCAN_MAGIC;
diff --git a/drivers/net/can/vcan.c b/drivers/net/can/vcan.c
index 39ca14b0585d..d8c8f9cc769f 100644
--- a/drivers/net/can/vcan.c
+++ b/drivers/net/can/vcan.c
@@ -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);
+	dev->can		= netdev_priv(dev);
 
 	/* set flags according to driver capabilities */
 	if (echo)
diff --git a/drivers/net/can/vxcan.c b/drivers/net/can/vxcan.c
index fa47bab510bb..0fea82935bf0 100644
--- a/drivers/net/can/vxcan.c
+++ b/drivers/net/can/vxcan.c
@@ -147,7 +147,7 @@ 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);
+	dev->can		= netdev_priv(dev) + ALIGN(sizeof(struct vxcan_priv), NETDEV_ALIGN);
 }
 
 /* forward declaration for rtnl_create_link() */
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 5b949076ed23..825220da2e4f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2153,6 +2153,10 @@ struct net_device {
 
 	/* protected by rtnl_lock */
 	struct bpf_xdp_entity	xdp_state[__MAX_XDP_MODE];
+
+#if IS_ENABLED(CONFIG_CAN)
+	struct can_ml_priv	*can;
+#endif
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 837bb8af0ec3..8651c8112a65 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -304,7 +304,7 @@ 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;
+		struct can_ml_priv *ml_priv = dev->can;
 		return &ml_priv->dev_rcv_lists;
 	} else {
 		return net->can.rx_alldev_list;
@@ -801,7 +801,7 @@ static int can_notifier(struct notifier_block *nb, unsigned long msg,
 
 	switch (msg) {
 	case NETDEV_REGISTER:
-		WARN(!dev->ml_priv,
+		WARN(!dev->can,
 		     "No CAN mid layer private allocated, please fix your driver and use alloc_candev()!\n");
 		break;
 	}
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index bb914d8b4216..62088074230d 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -140,7 +140,7 @@ 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_priv = ndev->can;
 
 	can_ml_priv->j1939_priv = priv;
 }
@@ -211,7 +211,7 @@ 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_priv = ndev->can;
 
 	if (!can_ml_priv)
 		return NULL;
diff --git a/net/can/j1939/socket.c b/net/can/j1939/socket.c
index f23966526a88..8010fbc8bd29 100644
--- a/net/can/j1939/socket.c
+++ b/net/can/j1939/socket.c
@@ -467,7 +467,7 @@ static int j1939_sk_bind(struct socket *sock, struct sockaddr *uaddr, int len)
 			goto out_release_sock;
 		}
 
-		if (!ndev->ml_priv) {
+		if (!ndev->can) {
 			netdev_warn_once(ndev,
 					 "No CAN mid layer private allocated, please fix your driver and use alloc_candev()!\n");
 			dev_put(ndev);
diff --git a/net/can/proc.c b/net/can/proc.c
index 5ea8695f507e..7d7a7f6d2cc9 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -322,8 +322,9 @@ 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);
+		if (dev->type == ARPHRD_CAN && dev->can)
+			can_rcvlist_proc_show_one(m, idx, dev,
+						  &dev->can->dev_rcv_lists);
 	}
 
 	rcu_read_unlock();
@@ -375,8 +376,8 @@ 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;
+		if (dev->type == ARPHRD_CAN && dev->can) {
+			dev_rcv_lists = &dev->can->dev_rcv_lists;
 			can_rcvlist_proc_show_array(m, dev, dev_rcv_lists->rx_sff,
 						    ARRAY_SIZE(dev_rcv_lists->rx_sff));
 		}
@@ -406,8 +407,8 @@ 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;
+		if (dev->type == ARPHRD_CAN && dev->can) {
+			dev_rcv_lists = &dev->can->dev_rcv_lists;
 			can_rcvlist_proc_show_array(m, dev, dev_rcv_lists->rx_eff,
 						    ARRAY_SIZE(dev_rcv_lists->rx_eff));
 		}
-- 
2.30.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ