Merge pull request #521 from ecsv/batadv-for-17.01

lede-17.01: batman-adv: Merge bugfixes from 2019.4
This commit is contained in:
Simon Wunderlich 2019-10-26 10:30:58 +02:00 committed by GitHub
commit f659697eff
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 843 additions and 0 deletions

View File

@ -0,0 +1,58 @@
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 12 Aug 2019 04:57:27 -0700
Subject: batman-adv: fix uninit-value in batadv_netlink_get_ifindex()
batadv_netlink_get_ifindex() needs to make sure user passed
a correct u32 attribute.
syzbot reported :
BUG: KMSAN: uninit-value in batadv_netlink_dump_hardif+0x70d/0x880 net/batman-adv/netlink.c:968
CPU: 1 PID: 11705 Comm: syz-executor888 Not tainted 5.1.0+ #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x191/0x1f0 lib/dump_stack.c:113
kmsan_report+0x130/0x2a0 mm/kmsan/kmsan.c:622
__msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:310
batadv_netlink_dump_hardif+0x70d/0x880 net/batman-adv/netlink.c:968
genl_lock_dumpit+0xc6/0x130 net/netlink/genetlink.c:482
netlink_dump+0xa84/0x1ab0 net/netlink/af_netlink.c:2253
__netlink_dump_start+0xa3a/0xb30 net/netlink/af_netlink.c:2361
genl_family_rcv_msg net/netlink/genetlink.c:550 [inline]
genl_rcv_msg+0xfc1/0x1a40 net/netlink/genetlink.c:627
netlink_rcv_skb+0x431/0x620 net/netlink/af_netlink.c:2486
genl_rcv+0x63/0x80 net/netlink/genetlink.c:638
netlink_unicast_kernel net/netlink/af_netlink.c:1311 [inline]
netlink_unicast+0xf3e/0x1020 net/netlink/af_netlink.c:1337
netlink_sendmsg+0x127e/0x12f0 net/netlink/af_netlink.c:1926
sock_sendmsg_nosec net/socket.c:651 [inline]
sock_sendmsg net/socket.c:661 [inline]
___sys_sendmsg+0xcc6/0x1200 net/socket.c:2260
__sys_sendmsg net/socket.c:2298 [inline]
__do_sys_sendmsg net/socket.c:2307 [inline]
__se_sys_sendmsg+0x305/0x460 net/socket.c:2305
__x64_sys_sendmsg+0x4a/0x70 net/socket.c:2305
do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:291
entry_SYSCALL_64_after_hwframe+0x63/0xe7
RIP: 0033:0x440209
Fixes: 55d368c3a57e ("batman-adv: netlink: hardif query")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/9b470b8a2b9ef4ce68d6e95febd3a0574be1ac14
diff --git a/net/batman-adv/netlink.c b/net/batman-adv/netlink.c
index 062738163bdce747b7f49c96d9180899bb15ea2f..e6852deebe1a07100ba2a6ac3631ab977797464c 100644
--- a/net/batman-adv/netlink.c
+++ b/net/batman-adv/netlink.c
@@ -110,7 +110,7 @@ batadv_netlink_get_ifindex(const struct nlmsghdr *nlh, int attrtype)
{
struct nlattr *attr = nlmsg_find_attr(nlh, GENL_HDRLEN, attrtype);
- return attr ? nla_get_u32(attr) : 0;
+ return (attr && nla_len(attr) == sizeof(u32)) ? nla_get_u32(attr) : 0;
}
/**

View File

@ -0,0 +1,74 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Fri, 23 Aug 2019 14:34:27 +0200
Subject: batman-adv: Only read OGM tvlv_len after buffer len check
Multiple batadv_ogm_packet can be stored in an skbuff. The functions
batadv_iv_ogm_send_to_if()/batadv_iv_ogm_receive() use
batadv_iv_ogm_aggr_packet() to check if there is another additional
batadv_ogm_packet in the skb or not before they continue processing the
packet.
The length for such an OGM is BATADV_OGM_HLEN +
batadv_ogm_packet->tvlv_len. The check must first check that at least
BATADV_OGM_HLEN bytes are available before it accesses tvlv_len (which is
part of the header. Otherwise it might try read outside of the currently
available skbuff to get the content of tvlv_len.
Fixes: 0b6aa0d43767 ("batman-adv: tvlv - basic infrastructure")
Reported-by: syzbot+355cab184197dbbfa384@syzkaller.appspotmail.com
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Acked-by: Antonio Quartulli <a@unstable.cc>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/07b6051ebcfaa7ea89b4f278eca2ff4070d29e56
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 7bfd0d7ef49df8e699f91e2b827b824aa3657c0d..5afc4e9646d622c6f924d4610ac58eb77404b37a 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -450,17 +450,23 @@ static u8 batadv_hop_penalty(u8 tq, const struct batadv_priv *bat_priv)
* batadv_iv_ogm_aggr_packet - checks if there is another OGM attached
* @buff_pos: current position in the skb
* @packet_len: total length of the skb
- * @tvlv_len: tvlv length of the previously considered OGM
+ * @ogm_packet: potential OGM in buffer
*
* Return: true if there is enough space for another OGM, false otherwise.
*/
-static bool batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len,
- __be16 tvlv_len)
+static bool
+batadv_iv_ogm_aggr_packet(int buff_pos, int packet_len,
+ const struct batadv_ogm_packet *ogm_packet)
{
int next_buff_pos = 0;
- next_buff_pos += buff_pos + BATADV_OGM_HLEN;
- next_buff_pos += ntohs(tvlv_len);
+ /* check if there is enough space for the header */
+ next_buff_pos += buff_pos + sizeof(*ogm_packet);
+ if (next_buff_pos > packet_len)
+ return false;
+
+ /* check if there is enough space for the optional TVLV */
+ next_buff_pos += ntohs(ogm_packet->tvlv_len);
return (next_buff_pos <= packet_len) &&
(next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
@@ -488,7 +494,7 @@ static void batadv_iv_ogm_send_to_if(struct batadv_forw_packet *forw_packet,
/* adjust all flags and log packets */
while (batadv_iv_ogm_aggr_packet(buff_pos, forw_packet->packet_len,
- batadv_ogm_packet->tvlv_len)) {
+ batadv_ogm_packet)) {
/* we might have aggregated direct link packets with an
* ordinary base packet
*/
@@ -1838,7 +1844,7 @@ static int batadv_iv_ogm_receive(struct sk_buff *skb,
/* unpack the aggregated packets and process them one by one */
while (batadv_iv_ogm_aggr_packet(ogm_offset, skb_headlen(skb),
- ogm_packet->tvlv_len)) {
+ ogm_packet)) {
batadv_iv_ogm_process(skb, ogm_offset, if_incoming);
ogm_offset += BATADV_OGM_HLEN;

View File

@ -0,0 +1,62 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Fri, 23 Aug 2019 14:34:28 +0200
Subject: batman-adv: Only read OGM2 tvlv_len after buffer len check
Multiple batadv_ogm2_packet can be stored in an skbuff. The functions
batadv_v_ogm_send_to_if() uses batadv_v_ogm_aggr_packet() to check if there
is another additional batadv_ogm2_packet in the skb or not before they
continue processing the packet.
The length for such an OGM2 is BATADV_OGM2_HLEN +
batadv_ogm2_packet->tvlv_len. The check must first check that at least
BATADV_OGM2_HLEN bytes are available before it accesses tvlv_len (which is
part of the header. Otherwise it might try read outside of the currently
available skbuff to get the content of tvlv_len.
Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/18f77da3761c5550f42a2d131f0fe5cac62e022d
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 38b9aab83fc0eaf63e3713d278482524253d5c1a..5f4fe1889053d4ea7624e4500dcefe2601371024 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -644,17 +644,23 @@ batadv_v_ogm_process_per_outif(struct batadv_priv *bat_priv,
* batadv_v_ogm_aggr_packet - checks if there is another OGM aggregated
* @buff_pos: current position in the skb
* @packet_len: total length of the skb
- * @tvlv_len: tvlv length of the previously considered OGM
+ * @ogm2_packet: potential OGM2 in buffer
*
* Return: true if there is enough space for another OGM, false otherwise.
*/
-static bool batadv_v_ogm_aggr_packet(int buff_pos, int packet_len,
- __be16 tvlv_len)
+static bool
+batadv_v_ogm_aggr_packet(int buff_pos, int packet_len,
+ const struct batadv_ogm2_packet *ogm2_packet)
{
int next_buff_pos = 0;
- next_buff_pos += buff_pos + BATADV_OGM2_HLEN;
- next_buff_pos += ntohs(tvlv_len);
+ /* check if there is enough space for the header */
+ next_buff_pos += buff_pos + sizeof(*ogm2_packet);
+ if (next_buff_pos > packet_len)
+ return false;
+
+ /* check if there is enough space for the optional TVLV */
+ next_buff_pos += ntohs(ogm2_packet->tvlv_len);
return (next_buff_pos <= packet_len) &&
(next_buff_pos <= BATADV_MAX_AGGREGATION_BYTES);
@@ -831,7 +837,7 @@ int batadv_v_ogm_packet_recv(struct sk_buff *skb,
ogm_packet = (struct batadv_ogm2_packet *)skb->data;
while (batadv_v_ogm_aggr_packet(ogm_offset, skb_headlen(skb),
- ogm_packet->tvlv_len)) {
+ ogm_packet)) {
batadv_v_ogm_process(skb, ogm_offset, if_incoming);
ogm_offset += BATADV_OGM2_HLEN;

View File

@ -0,0 +1,117 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Thu, 3 Oct 2019 17:02:01 +0200
Subject: batman-adv: Avoid free/alloc race when handling OGM2 buffer
A B.A.T.M.A.N. V virtual interface has an OGM2 packet buffer which is
initialized using data from the RTNL lock protected netdevice notifier and
other rtnetlink related hooks. It is sent regularly via various slave
interfaces of the batadv virtual interface and in this process also
modified (realloced) to integrate additional state information via TVLV
containers.
It must be avoided that the worker item is executed without a common lock
with the netdevice notifier/rtnetlink helpers. Otherwise it can either
happen that half modified data is sent out or the functions modifying the
OGM2 buffer try to access already freed memory regions.
Fixes: 632835348e65 ("batman-adv: OGMv2 - add basic infrastructure")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/14ee24576213ff02272b7f8d975c7c61d5448aa2
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index 5f4fe1889053d4ea7624e4500dcefe2601371024..f5feaa8c4fd228228fea519771e2c9e123b10345 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -32,6 +32,7 @@
#include <linux/random.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
+#include <linux/rtnetlink.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
#include <linux/stddef.h>
@@ -127,14 +128,12 @@ static void batadv_v_ogm_send_to_if(struct sk_buff *skb,
}
/**
- * batadv_v_ogm_send - periodic worker broadcasting the own OGM
- * @work: work queue item
+ * batadv_v_ogm_send_softif() - periodic worker broadcasting the own OGM
+ * @bat_priv: the bat priv with all the soft interface information
*/
-static void batadv_v_ogm_send(struct work_struct *work)
+static void batadv_v_ogm_send_softif(struct batadv_priv *bat_priv)
{
struct batadv_hard_iface *hard_iface;
- struct batadv_priv_bat_v *bat_v;
- struct batadv_priv *bat_priv;
struct batadv_ogm2_packet *ogm_packet;
struct sk_buff *skb, *skb_tmp;
unsigned char *ogm_buff, *pkt_buff;
@@ -142,8 +141,7 @@ static void batadv_v_ogm_send(struct work_struct *work)
u16 tvlv_len = 0;
int ret;
- bat_v = container_of(work, struct batadv_priv_bat_v, ogm_wq.work);
- bat_priv = container_of(bat_v, struct batadv_priv, bat_v);
+ ASSERT_RTNL();
if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING)
goto out;
@@ -235,6 +233,22 @@ out:
return;
}
+/**
+ * batadv_v_ogm_send() - periodic worker broadcasting the own OGM
+ * @work: work queue item
+ */
+static void batadv_v_ogm_send(struct work_struct *work)
+{
+ struct batadv_priv_bat_v *bat_v;
+ struct batadv_priv *bat_priv;
+
+ rtnl_lock();
+ bat_v = container_of(work, struct batadv_priv_bat_v, ogm_wq.work);
+ bat_priv = container_of(bat_v, struct batadv_priv, bat_v);
+ batadv_v_ogm_send_softif(bat_priv);
+ rtnl_unlock();
+}
+
/**
* batadv_v_ogm_iface_enable - prepare an interface for B.A.T.M.A.N. V
* @hard_iface: the interface to prepare
@@ -261,6 +275,8 @@ void batadv_v_ogm_primary_iface_set(struct batadv_hard_iface *primary_iface)
struct batadv_priv *bat_priv = netdev_priv(primary_iface->soft_iface);
struct batadv_ogm2_packet *ogm_packet;
+ ASSERT_RTNL();
+
if (!bat_priv->bat_v.ogm_buff)
return;
@@ -870,6 +886,8 @@ int batadv_v_ogm_init(struct batadv_priv *bat_priv)
unsigned char *ogm_buff;
u32 random_seqno;
+ ASSERT_RTNL();
+
bat_priv->bat_v.ogm_buff_len = BATADV_OGM2_HLEN;
ogm_buff = kzalloc(bat_priv->bat_v.ogm_buff_len, GFP_ATOMIC);
if (!ogm_buff)
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 5137d859694c28f60cad33325127617c047412ff..bf33f04e055bbf288ce6800d0b93da1b4cd9b70d 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -984,8 +984,8 @@ struct batadv_softif_vlan {
/**
* struct batadv_priv_bat_v - B.A.T.M.A.N. V per soft-interface private data
- * @ogm_buff: buffer holding the OGM packet
- * @ogm_buff_len: length of the OGM packet buffer
+ * @ogm_buff: buffer holding the OGM packet. rtnl protected
+ * @ogm_buff_len: length of the OGM packet buffer. rtnl protected
* @ogm_seqno: OGM sequence number - used to identify each OGM
* @ogm_wq: workqueue used to schedule OGM transmissions
*/

View File

@ -0,0 +1,134 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Thu, 3 Oct 2019 17:02:01 +0200
Subject: batman-adv: Avoid free/alloc race when handling OGM buffer
Each slave interface of an B.A.T.M.A.N. IV virtual interface has an OGM
packet buffer which is initialized using data from the RTNL lock protected
netdevice notifier and other rtnetlink related hooks. It is sent regularly
via various slave interfaces of the batadv virtual interface and in this
process also modified (realloced) to integrate additional state information
via TVLV containers.
It must be avoided that the worker item is executed without a common lock
with the netdevice notifier/rtnetlink helpers. Otherwise it can either
happen that half modified/freed data is sent out or functions modifying the
OGM buffer try to access already freed memory regions.
Reported-by: syzbot+0cc629f19ccb8534935b@syzkaller.appspotmail.com
Fixes: ea6f8d42a595 ("batman-adv: move /proc interface handling to /sys")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/9b8ceef26c697d0c8319748428944c3339a498dc
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 5afc4e9646d622c6f924d4610ac58eb77404b37a..54c5be029b7418bc97d87e7642e258bfed120874 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -41,6 +41,7 @@
#include <linux/random.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
+#include <linux/rtnetlink.h>
#include <linux/seq_file.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
@@ -366,6 +367,8 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface)
unsigned char *ogm_buff;
u32 random_seqno;
+ ASSERT_RTNL();
+
/* randomize initial seqno to avoid collision */
get_random_bytes(&random_seqno, sizeof(random_seqno));
atomic_set(&hard_iface->bat_iv.ogm_seqno, random_seqno);
@@ -390,6 +393,8 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface)
static void batadv_iv_ogm_iface_disable(struct batadv_hard_iface *hard_iface)
{
+ ASSERT_RTNL();
+
kfree(hard_iface->bat_iv.ogm_buff);
hard_iface->bat_iv.ogm_buff = NULL;
}
@@ -399,6 +404,8 @@ static void batadv_iv_ogm_iface_update_mac(struct batadv_hard_iface *hard_iface)
struct batadv_ogm_packet *batadv_ogm_packet;
unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff;
+ ASSERT_RTNL();
+
batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff;
ether_addr_copy(batadv_ogm_packet->orig,
hard_iface->net_dev->dev_addr);
@@ -412,6 +419,8 @@ batadv_iv_ogm_primary_iface_set(struct batadv_hard_iface *hard_iface)
struct batadv_ogm_packet *batadv_ogm_packet;
unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff;
+ ASSERT_RTNL();
+
batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff;
batadv_ogm_packet->ttl = BATADV_TTL;
}
@@ -923,6 +932,8 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface)
u16 tvlv_len = 0;
unsigned long send_time;
+ ASSERT_RTNL();
+
if ((hard_iface->if_status == BATADV_IF_NOT_IN_USE) ||
(hard_iface->if_status == BATADV_IF_TO_BE_REMOVED))
return;
@@ -1777,16 +1788,12 @@ static void batadv_iv_ogm_process(const struct sk_buff *skb, int ogm_offset,
batadv_orig_node_put(orig_node);
}
-static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
+static void
+batadv_iv_send_outstanding_forw_packet(struct batadv_forw_packet *forw_packet)
{
- struct delayed_work *delayed_work;
- struct batadv_forw_packet *forw_packet;
struct batadv_priv *bat_priv;
bool dropped = false;
- delayed_work = to_delayed_work(work);
- forw_packet = container_of(delayed_work, struct batadv_forw_packet,
- delayed_work);
bat_priv = netdev_priv(forw_packet->if_incoming->soft_iface);
if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING) {
@@ -1815,6 +1822,20 @@ out:
batadv_forw_packet_free(forw_packet, dropped);
}
+static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
+{
+ struct delayed_work *delayed_work;
+ struct batadv_forw_packet *forw_packet;
+
+ delayed_work = to_delayed_work(work);
+ forw_packet = container_of(delayed_work, struct batadv_forw_packet,
+ delayed_work);
+
+ rtnl_lock();
+ batadv_iv_send_outstanding_forw_packet(forw_packet);
+ rtnl_unlock();
+}
+
static int batadv_iv_ogm_receive(struct sk_buff *skb,
struct batadv_hard_iface *if_incoming)
{
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index bf33f04e055bbf288ce6800d0b93da1b4cd9b70d..8906e551c85991e6b085108cf6f2b734b48fc231 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -78,8 +78,8 @@ enum batadv_dhcp_recipient {
/**
* struct batadv_hard_iface_bat_iv - per hard-interface B.A.T.M.A.N. IV data
- * @ogm_buff: buffer holding the OGM packet
- * @ogm_buff_len: length of the OGM packet buffer
+ * @ogm_buff: buffer holding the OGM packet. rtnl protected
+ * @ogm_buff_len: length of the OGM packet buffer. rtnl protected
* @ogm_seqno: OGM sequence number - used to identify each OGM
*/
struct batadv_hard_iface_bat_iv {

View File

@ -0,0 +1,137 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Sun, 13 Oct 2019 21:03:06 +0200
Subject: batman-adv: Introduce own OGM2 buffer mutex
Only a single function is currently automatically locked by the rtnl_lock
because (unlike B.A.T.M.A.N. IV) the OGM2 buffer is independent of the hard
interfaces on which it will be transmitted. A private mutex can be used
instead to avoid unnecessary delays which would have been introduced by the
global lock.
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/8069c581f9097f1f9398f2d49047a1dab8093821
diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c
index f5feaa8c4fd228228fea519771e2c9e123b10345..a9240a0bedad109aba58e30038fe91a421ab4126 100644
--- a/net/batman-adv/bat_v_ogm.c
+++ b/net/batman-adv/bat_v_ogm.c
@@ -28,11 +28,12 @@
#include <linux/kernel.h>
#include <linux/kref.h>
#include <linux/list.h>
+#include <linux/lockdep.h>
+#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/random.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
-#include <linux/rtnetlink.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
#include <linux/stddef.h>
@@ -141,7 +142,7 @@ static void batadv_v_ogm_send_softif(struct batadv_priv *bat_priv)
u16 tvlv_len = 0;
int ret;
- ASSERT_RTNL();
+ lockdep_assert_held(&bat_priv->bat_v.ogm_buff_mutex);
if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING)
goto out;
@@ -242,11 +243,12 @@ static void batadv_v_ogm_send(struct work_struct *work)
struct batadv_priv_bat_v *bat_v;
struct batadv_priv *bat_priv;
- rtnl_lock();
bat_v = container_of(work, struct batadv_priv_bat_v, ogm_wq.work);
bat_priv = container_of(bat_v, struct batadv_priv, bat_v);
+
+ mutex_lock(&bat_priv->bat_v.ogm_buff_mutex);
batadv_v_ogm_send_softif(bat_priv);
- rtnl_unlock();
+ mutex_unlock(&bat_priv->bat_v.ogm_buff_mutex);
}
/**
@@ -275,13 +277,15 @@ void batadv_v_ogm_primary_iface_set(struct batadv_hard_iface *primary_iface)
struct batadv_priv *bat_priv = netdev_priv(primary_iface->soft_iface);
struct batadv_ogm2_packet *ogm_packet;
- ASSERT_RTNL();
-
+ mutex_lock(&bat_priv->bat_v.ogm_buff_mutex);
if (!bat_priv->bat_v.ogm_buff)
- return;
+ goto unlock;
ogm_packet = (struct batadv_ogm2_packet *)bat_priv->bat_v.ogm_buff;
ether_addr_copy(ogm_packet->orig, primary_iface->net_dev->dev_addr);
+
+unlock:
+ mutex_unlock(&bat_priv->bat_v.ogm_buff_mutex);
}
/**
@@ -886,8 +890,6 @@ int batadv_v_ogm_init(struct batadv_priv *bat_priv)
unsigned char *ogm_buff;
u32 random_seqno;
- ASSERT_RTNL();
-
bat_priv->bat_v.ogm_buff_len = BATADV_OGM2_HLEN;
ogm_buff = kzalloc(bat_priv->bat_v.ogm_buff_len, GFP_ATOMIC);
if (!ogm_buff)
@@ -906,6 +908,8 @@ int batadv_v_ogm_init(struct batadv_priv *bat_priv)
atomic_set(&bat_priv->bat_v.ogm_seqno, random_seqno);
INIT_DELAYED_WORK(&bat_priv->bat_v.ogm_wq, batadv_v_ogm_send);
+ mutex_init(&bat_priv->bat_v.ogm_buff_mutex);
+
return 0;
}
@@ -917,7 +921,11 @@ void batadv_v_ogm_free(struct batadv_priv *bat_priv)
{
cancel_delayed_work_sync(&bat_priv->bat_v.ogm_wq);
+ mutex_lock(&bat_priv->bat_v.ogm_buff_mutex);
+
kfree(bat_priv->bat_v.ogm_buff);
bat_priv->bat_v.ogm_buff = NULL;
bat_priv->bat_v.ogm_buff_len = 0;
+
+ mutex_unlock(&bat_priv->bat_v.ogm_buff_mutex);
}
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 8906e551c85991e6b085108cf6f2b734b48fc231..57df7c68dfee8d6bed72edbd044d95016a5dd75e 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -27,6 +27,7 @@
#include <linux/compiler.h>
#include <linux/if_ether.h>
#include <linux/kref.h>
+#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/netlink.h>
#include <linux/sched.h> /* for linux/wait.h */
@@ -984,15 +985,17 @@ struct batadv_softif_vlan {
/**
* struct batadv_priv_bat_v - B.A.T.M.A.N. V per soft-interface private data
- * @ogm_buff: buffer holding the OGM packet. rtnl protected
- * @ogm_buff_len: length of the OGM packet buffer. rtnl protected
+ * @ogm_buff: buffer holding the OGM packet
+ * @ogm_buff_len: length of the OGM packet buffer
* @ogm_seqno: OGM sequence number - used to identify each OGM
+ * @ogm_buff_mutex: lock protecting ogm_buff and ogm_buff_len
* @ogm_wq: workqueue used to schedule OGM transmissions
*/
struct batadv_priv_bat_v {
unsigned char *ogm_buff;
int ogm_buff_len;
atomic_t ogm_seqno;
+ struct mutex ogm_buff_mutex;
struct delayed_work ogm_wq;
};

View File

@ -0,0 +1,261 @@
From: Sven Eckelmann <sven@narfation.org>
Date: Sun, 13 Oct 2019 21:03:07 +0200
Subject: batman-adv: Avoid OGM workqueue synchronous cancel deadlock
batadv_forw_packet_list_free can be called when an interface is being
disabled. Under this circumstance, the rntl_lock will be held and while it
calls cancel_delayed_work_sync.
cancel_delayed_work_sync will stop the execution of the current context
when the work item is currently processed. It can now happen that the
cancel_delayed_work_sync was called when rtnl_lock was already called in
batadv_iv_send_outstanding_bat_ogm_packet or when it was in the process of
calling it. In this case, batadv_iv_send_outstanding_bat_ogm_packet waits
for the lock and cancel_delayed_work_sync (which holds the rtnl_lock) is
waiting for batadv_iv_send_outstanding_bat_ogm_packet to finish.
This can only be avoided by not using (conflicting) blocking locks while
cancel_delayed_work_sync is called. It also has the benefit that the
ogm scheduling functionality can avoid unnecessary delays which can be
introduced by a global lock.
Fixes: 9b8ceef26c69 ("batman-adv: Avoid free/alloc race when handling OGM buffer")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/d3be478f1aa27b47f61c4a62e18eb063d47c9168
diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c
index 54c5be029b7418bc97d87e7642e258bfed120874..3fb3feaf5e462f7263754b81a06fc877aae3f10d 100644
--- a/net/batman-adv/bat_iv_ogm.c
+++ b/net/batman-adv/bat_iv_ogm.c
@@ -34,6 +34,7 @@
#include <linux/kref.h>
#include <linux/list.h>
#include <linux/lockdep.h>
+#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/netlink.h>
#include <linux/pkt_sched.h>
@@ -41,7 +42,6 @@
#include <linux/random.h>
#include <linux/rculist.h>
#include <linux/rcupdate.h>
-#include <linux/rtnetlink.h>
#include <linux/seq_file.h>
#include <linux/skbuff.h>
#include <linux/slab.h>
@@ -367,7 +367,7 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface)
unsigned char *ogm_buff;
u32 random_seqno;
- ASSERT_RTNL();
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex);
/* randomize initial seqno to avoid collision */
get_random_bytes(&random_seqno, sizeof(random_seqno));
@@ -375,8 +375,10 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface)
hard_iface->bat_iv.ogm_buff_len = BATADV_OGM_HLEN;
ogm_buff = kmalloc(hard_iface->bat_iv.ogm_buff_len, GFP_ATOMIC);
- if (!ogm_buff)
+ if (!ogm_buff) {
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
return -ENOMEM;
+ }
hard_iface->bat_iv.ogm_buff = ogm_buff;
@@ -388,41 +390,59 @@ static int batadv_iv_ogm_iface_enable(struct batadv_hard_iface *hard_iface)
batadv_ogm_packet->reserved = 0;
batadv_ogm_packet->tq = BATADV_TQ_MAX_VALUE;
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
+
return 0;
}
static void batadv_iv_ogm_iface_disable(struct batadv_hard_iface *hard_iface)
{
- ASSERT_RTNL();
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex);
kfree(hard_iface->bat_iv.ogm_buff);
hard_iface->bat_iv.ogm_buff = NULL;
+
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
}
static void batadv_iv_ogm_iface_update_mac(struct batadv_hard_iface *hard_iface)
{
struct batadv_ogm_packet *batadv_ogm_packet;
- unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff;
+ void *ogm_buff;
- ASSERT_RTNL();
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex);
- batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff;
+ ogm_buff = hard_iface->bat_iv.ogm_buff;
+ if (!ogm_buff)
+ goto unlock;
+
+ batadv_ogm_packet = ogm_buff;
ether_addr_copy(batadv_ogm_packet->orig,
hard_iface->net_dev->dev_addr);
ether_addr_copy(batadv_ogm_packet->prev_sender,
hard_iface->net_dev->dev_addr);
+
+unlock:
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
}
static void
batadv_iv_ogm_primary_iface_set(struct batadv_hard_iface *hard_iface)
{
struct batadv_ogm_packet *batadv_ogm_packet;
- unsigned char *ogm_buff = hard_iface->bat_iv.ogm_buff;
+ void *ogm_buff;
- ASSERT_RTNL();
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex);
- batadv_ogm_packet = (struct batadv_ogm_packet *)ogm_buff;
+ ogm_buff = hard_iface->bat_iv.ogm_buff;
+ if (!ogm_buff)
+ goto unlock;
+
+ batadv_ogm_packet = ogm_buff;
batadv_ogm_packet->ttl = BATADV_TTL;
+
+unlock:
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
}
/* when do we schedule our own ogm to be sent */
@@ -921,7 +941,11 @@ batadv_iv_ogm_slide_own_bcast_window(struct batadv_hard_iface *hard_iface)
}
}
-static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface)
+/**
+ * batadv_iv_ogm_schedule_buff() - schedule submission of hardif ogm buffer
+ * @hard_iface: interface whose ogm buffer should be transmitted
+ */
+static void batadv_iv_ogm_schedule_buff(struct batadv_hard_iface *hard_iface)
{
struct batadv_priv *bat_priv = netdev_priv(hard_iface->soft_iface);
unsigned char **ogm_buff = &hard_iface->bat_iv.ogm_buff;
@@ -932,11 +956,7 @@ static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface)
u16 tvlv_len = 0;
unsigned long send_time;
- ASSERT_RTNL();
-
- if ((hard_iface->if_status == BATADV_IF_NOT_IN_USE) ||
- (hard_iface->if_status == BATADV_IF_TO_BE_REMOVED))
- return;
+ lockdep_assert_held(&hard_iface->bat_iv.ogm_buff_mutex);
/* the interface gets activated here to avoid race conditions between
* the moment of activating the interface in
@@ -1004,6 +1024,17 @@ out:
batadv_hardif_put(primary_if);
}
+static void batadv_iv_ogm_schedule(struct batadv_hard_iface *hard_iface)
+{
+ if (hard_iface->if_status == BATADV_IF_NOT_IN_USE ||
+ hard_iface->if_status == BATADV_IF_TO_BE_REMOVED)
+ return;
+
+ mutex_lock(&hard_iface->bat_iv.ogm_buff_mutex);
+ batadv_iv_ogm_schedule_buff(hard_iface);
+ mutex_unlock(&hard_iface->bat_iv.ogm_buff_mutex);
+}
+
/**
* batadv_iv_ogm_orig_update - use OGM to update corresponding data in an
* originator
@@ -1788,12 +1819,16 @@ static void batadv_iv_ogm_process(const struct sk_buff *skb, int ogm_offset,
batadv_orig_node_put(orig_node);
}
-static void
-batadv_iv_send_outstanding_forw_packet(struct batadv_forw_packet *forw_packet)
+static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
{
+ struct delayed_work *delayed_work;
+ struct batadv_forw_packet *forw_packet;
struct batadv_priv *bat_priv;
bool dropped = false;
+ delayed_work = to_delayed_work(work);
+ forw_packet = container_of(delayed_work, struct batadv_forw_packet,
+ delayed_work);
bat_priv = netdev_priv(forw_packet->if_incoming->soft_iface);
if (atomic_read(&bat_priv->mesh_state) == BATADV_MESH_DEACTIVATING) {
@@ -1822,20 +1857,6 @@ out:
batadv_forw_packet_free(forw_packet, dropped);
}
-static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work)
-{
- struct delayed_work *delayed_work;
- struct batadv_forw_packet *forw_packet;
-
- delayed_work = to_delayed_work(work);
- forw_packet = container_of(delayed_work, struct batadv_forw_packet,
- delayed_work);
-
- rtnl_lock();
- batadv_iv_send_outstanding_forw_packet(forw_packet);
- rtnl_unlock();
-}
-
static int batadv_iv_ogm_receive(struct sk_buff *skb,
struct batadv_hard_iface *if_incoming)
{
diff --git a/net/batman-adv/hard-interface.c b/net/batman-adv/hard-interface.c
index 6969f580d0bfd0428f1c6985eaec8bbbf5a0d38b..72b63117fe47c4cb91eb8512b4f44d6b0bc670c9 100644
--- a/net/batman-adv/hard-interface.c
+++ b/net/batman-adv/hard-interface.c
@@ -29,6 +29,7 @@
#include <linux/kernel.h>
#include <linux/kref.h>
#include <linux/list.h>
+#include <linux/mutex.h>
#include <linux/netdevice.h>
#include <linux/printk.h>
#include <linux/rculist.h>
@@ -896,6 +897,7 @@ batadv_hardif_add_interface(struct net_device *net_dev)
INIT_LIST_HEAD(&hard_iface->list);
INIT_HLIST_HEAD(&hard_iface->neigh_list);
+ mutex_init(&hard_iface->bat_iv.ogm_buff_mutex);
spin_lock_init(&hard_iface->neigh_list_lock);
kref_init(&hard_iface->refcount);
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 57df7c68dfee8d6bed72edbd044d95016a5dd75e..ee1a3243b160b5ca87e1a42654e579d7358aab6b 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -79,14 +79,16 @@ enum batadv_dhcp_recipient {
/**
* struct batadv_hard_iface_bat_iv - per hard-interface B.A.T.M.A.N. IV data
- * @ogm_buff: buffer holding the OGM packet. rtnl protected
- * @ogm_buff_len: length of the OGM packet buffer. rtnl protected
+ * @ogm_buff: buffer holding the OGM packet
+ * @ogm_buff_len: length of the OGM packet buffer
* @ogm_seqno: OGM sequence number - used to identify each OGM
+ * @ogm_buff_mutex: lock protecting ogm_buff and ogm_buff_len
*/
struct batadv_hard_iface_bat_iv {
unsigned char *ogm_buff;
int ogm_buff_len;
atomic_t ogm_seqno;
+ struct mutex ogm_buff_mutex;
};
/**