From patchwork Thu Oct 3 16:43:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 18034 X-Patchwork-Delegate: sw@simonwunderlich.de Return-Path: Received: from dvalin.narfation.org (dvalin.narfation.org [213.160.73.56]) by open-mesh.org (Postfix) with ESMTPS id 42C8E80162 for ; Thu, 3 Oct 2019 18:43:58 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=narfation.org; s=20121; t=1570121037; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=3edw67EmbB+O7xoYQir07L/ETezyucEvZwra6DiC6v4=; b=GyTkgex1KQ3pT/BgT5Y5y331VtHg0o1HKzbvhzNcrbqU55ykTjoJqnorBxxvMv4B69kMup deoi62EejQzLrLL/CWHsPVC8smyfr7SJT1GYTQvn0OPBnqYZvAX0EVQ7jVKHGAc3+6cbvF QlBQtkmgruF8Ro5kUx/a7Xen1kjnDIc= From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Subject: [PATCH maint 1/2] batman-adv: Avoid free/alloc race when handling OGM2 buffer Date: Thu, 3 Oct 2019 18:43:52 +0200 Message-Id: <20191003164353.14095-1-sven@narfation.org> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=narfation.org; s=20121; t=1570121037; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=3edw67EmbB+O7xoYQir07L/ETezyucEvZwra6DiC6v4=; b=kiy7RG2jLmhgCqmWD+2M/sPeZSmbzqgOxN/qzUroMgBxxbTKEHcy9rGrovqNs/XZawIGk9 lr2YgsoTWCNVSxD47vaq81lw9yNrNQt2CtOetnH85hVrpOnxENceJvsTt+8XGD2eUjClhF 08kSCSrdPxwWMOlX0zC8rqD7bchM5zc= ARC-Seal: i=1; s=20121; d=narfation.org; t=1570121037; a=rsa-sha256; cv=none; b=zIjucK26MJRBHDtSRkwRxLeB81xDIm4IWiczNlPQ8k8GBvaENk7aa6SD9/cOQ5uMi4ik1Q MWNPbEzmknxGoTlQ4ZCc+dGHhPzW6Z7keTzdheJsAEbuAoLxqr+azJAgDnEonoDpdwjf7f Nc00ZuzXTlJjlR/ao2BXXmbFrJc0dZw= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=sven smtp.mailfrom=sven@narfation.org X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Oct 2019 16:43:59 -0000 A B.A.T.M.A.N. V virtual interface has an OGM2 packet buffer which is initialized using data from the RTNL 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 --- net/batman-adv/bat_v_ogm.c | 32 +++++++++++++++++++++++++------- net/batman-adv/types.h | 4 ++-- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c index dc4f7430..24d00a97 100644 --- a/net/batman-adv/bat_v_ogm.c +++ b/net/batman-adv/bat_v_ogm.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -256,14 +257,12 @@ static void batadv_v_ogm_queue_on_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; @@ -271,8 +270,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; @@ -363,6 +361,22 @@ static void batadv_v_ogm_send(struct work_struct *work) 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_aggr_work() - OGM queue periodic task per interface * @work: work queue item @@ -424,6 +438,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; @@ -1032,6 +1048,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 be7c02aa..bb7b4ab1 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -1530,10 +1530,10 @@ struct batadv_softif_vlan { * struct batadv_priv_bat_v - B.A.T.M.A.N. V per soft-interface private data */ struct batadv_priv_bat_v { - /** @ogm_buff: buffer holding the OGM packet */ + /** @ogm_buff: buffer holding the OGM packet. rtnl protected */ unsigned char *ogm_buff; - /** @ogm_buff_len: length of the OGM packet buffer */ + /** @ogm_buff_len: length of the OGM packet buffer. rtnl protected */ int ogm_buff_len; /** @ogm_seqno: OGM sequence number - used to identify each OGM */ From patchwork Thu Oct 3 16:43:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sven Eckelmann X-Patchwork-Id: 18035 X-Patchwork-Delegate: sw@simonwunderlich.de Return-Path: Received: from dvalin.narfation.org (dvalin.narfation.org [IPv6:2a00:17d8:100::8b1]) by open-mesh.org (Postfix) with ESMTPS id 07FD780162 for ; Thu, 3 Oct 2019 18:44:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=narfation.org; s=20121; t=1570121066; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=y20+glqyAQ+F09PR9Da7GCZo6Yn/xoaXLNT5jVrImSA=; b=i/PS9ouAUFncCvD1cLsY+Me7pPhsRuqJLRg/YK+K2Jzq1xX6ofr7PrsMN2jotLWyPwyC5H pYHP/+18FGsEo8FLm8+4AYQb/WrVQq/AfuROBtBM2FT7Ce0c2bn9Qk4rgWxUNSXHM8ytJu q2gxv0dFWgM2Qupu9SGVMvztor/YP4w= From: Sven Eckelmann To: b.a.t.m.a.n@lists.open-mesh.org Cc: Sven Eckelmann , syzbot+0cc629f19ccb8534935b@syzkaller.appspotmail.com Subject: [PATCH maint 2/2] batman-adv: Avoid free/alloc race when handling OGM buffer Date: Thu, 3 Oct 2019 18:43:53 +0200 Message-Id: <20191003164353.14095-2-sven@narfation.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191003164353.14095-1-sven@narfation.org> References: <20191003164353.14095-1-sven@narfation.org> MIME-Version: 1.0 ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=narfation.org; s=20121; t=1570121066; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=y20+glqyAQ+F09PR9Da7GCZo6Yn/xoaXLNT5jVrImSA=; b=QrIQ4aebE1EDjDnXeDkrVzZTEVXgZZY1KlbCyxW1koi14G3QysGkho9Z5Xw6AEQMW1umJW 3rpnrZ7/5MPzhFDXUB6wcZ8/toOH9HSBM1AutbF9JDPznJJCAEEjuahRDTVl5vJWH9pEae Klb7rdN8ZG14eGjyGLx+iwWlpTPrcyM= ARC-Seal: i=1; s=20121; d=narfation.org; t=1570121066; a=rsa-sha256; cv=none; b=1JqQKr7nD0wMo7FbSaCrswhM6NX1AyMzC1wYz1qIFqZhr/l8SRzHFGyVVam6YoFn+OeDz4 boEfDdf5/EYBQ8/+amV+9cXvRXcnusuotDEtzIs62P2BOG4V4NYdwaz5+dchck6N+n6Ul2 tDFX//IdA0QIr/Ov5NTZxkUh+F/IlVo= ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=sven smtp.mailfrom=sven@narfation.org X-BeenThere: b.a.t.m.a.n@lists.open-mesh.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The list for a Better Approach To Mobile Ad-hoc Networking List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 03 Oct 2019 16:44:27 -0000 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 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 --- See https://www.open-mesh.org/issues/400 net/batman-adv/bat_iv_ogm.c | 33 +++++++++++++++++++++++++++------ net/batman-adv/types.h | 4 ++-- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/net/batman-adv/bat_iv_ogm.c b/net/batman-adv/bat_iv_ogm.c index d78938e3..e20c3813 100644 --- a/net/batman-adv/bat_iv_ogm.c +++ b/net/batman-adv/bat_iv_ogm.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -193,6 +194,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); @@ -217,6 +220,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; } @@ -226,6 +231,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); @@ -239,6 +246,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; } @@ -753,6 +762,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; @@ -1643,16 +1654,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) { @@ -1681,6 +1688,20 @@ static void batadv_iv_send_outstanding_bat_ogm_packet(struct work_struct *work) 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 bb7b4ab1..437d92e1 100644 --- a/net/batman-adv/types.h +++ b/net/batman-adv/types.h @@ -73,10 +73,10 @@ enum batadv_dhcp_recipient { * struct batadv_hard_iface_bat_iv - per hard-interface B.A.T.M.A.N. IV data */ struct batadv_hard_iface_bat_iv { - /** @ogm_buff: buffer holding the OGM packet */ + /** @ogm_buff: buffer holding the OGM packet. rtnl protected */ unsigned char *ogm_buff; - /** @ogm_buff_len: length of the OGM packet buffer */ + /** @ogm_buff_len: length of the OGM packet buffer. rtnl protected */ int ogm_buff_len; /** @ogm_seqno: OGM sequence number - used to identify each OGM */