From 719709a03efa7b4e0f59fd55e15ee78f597bb03c Mon Sep 17 00:00:00 2001 From: Sven Eckelmann Date: Fri, 24 Apr 2020 20:52:25 +0200 Subject: [PATCH] batman-adv: Merge bugfixes from 2020.1 * fix batadv_nc_random_weight_tq * Fix refcnt leak in batadv_show_throughput_override * Fix refcnt leak in batadv_store_throughput_override * Fix refcnt leak in batadv_v_ogm_process Signed-off-by: Sven Eckelmann --- batman-adv/Makefile | 2 +- ...n-adv-fix-batadv_nc_random_weight_tq.patch | 59 +++++++++++++++++++ ...efcnt-leak-in-batadv_show_throughput.patch | 38 ++++++++++++ ...efcnt-leak-in-batadv_store_throughpu.patch | 39 ++++++++++++ ...-refcnt-leak-in-batadv_v_ogm_process.patch | 39 ++++++++++++ 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 batman-adv/patches/0042-batman-adv-fix-batadv_nc_random_weight_tq.patch create mode 100644 batman-adv/patches/0043-batman-adv-Fix-refcnt-leak-in-batadv_show_throughput.patch create mode 100644 batman-adv/patches/0044-batman-adv-Fix-refcnt-leak-in-batadv_store_throughpu.patch create mode 100644 batman-adv/patches/0045-batman-adv-Fix-refcnt-leak-in-batadv_v_ogm_process.patch diff --git a/batman-adv/Makefile b/batman-adv/Makefile index bcfe2e6..75bf532 100644 --- a/batman-adv/Makefile +++ b/batman-adv/Makefile @@ -10,7 +10,7 @@ include $(TOPDIR)/rules.mk PKG_NAME:=batman-adv PKG_VERSION:=2018.1 -PKG_RELEASE:=10 +PKG_RELEASE:=11 PKG_HASH:=b866b28dbbe5c9238abbdf5abbc30fc526dea56898ce4c1bd76d5c017843048b PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz diff --git a/batman-adv/patches/0042-batman-adv-fix-batadv_nc_random_weight_tq.patch b/batman-adv/patches/0042-batman-adv-fix-batadv_nc_random_weight_tq.patch new file mode 100644 index 0000000..d3f356b --- /dev/null +++ b/batman-adv/patches/0042-batman-adv-fix-batadv_nc_random_weight_tq.patch @@ -0,0 +1,59 @@ +From: George Spelvin +Date: Sun, 8 Mar 2020 09:44:59 -0400 +Subject: batman-adv: fix batadv_nc_random_weight_tq + +and change to pseudorandom numbers, as this is a traffic dithering +operation that doesn't need crypto-grade. + +The previous code operated in 4 steps: + +1. Generate a random byte 0 <= rand_tq <= 255 +2. Multiply it by BATADV_TQ_MAX_VALUE - tq +3. Divide by 255 (= BATADV_TQ_MAX_VALUE) +4. Return BATADV_TQ_MAX_VALUE - rand_tq + +This would apperar to scale (BATADV_TQ_MAX_VALUE - tq) by a random +value between 0/255 and 255/255. + +But! The intermediate value between steps 3 and 4 is stored in a u8 +variable. So it's truncated, and most of the time, is less than 255, after +which the division produces 0. Specifically, if tq is odd, the product is +always even, and can never be 255. If tq is even, there's exactly one +random byte value that will produce a product byte of 255. + +Thus, the return value is 255 (511/512 of the time) or 254 (1/512 +of the time). + +If we assume that the truncation is a bug, and the code is meant to scale +the input, a simpler way of looking at it is that it's returning a random +value between tq and BATADV_TQ_MAX_VALUE, inclusive. + +Well, we have an optimized function for doing just that. + +Fixes: c3289f3650d3 ("batman-adv: network coding - code and transmit packets if possible") +Signed-off-by: George Spelvin +Signed-off-by: Sven Eckelmann + +Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/db48c60b0edb995450ee846157364bd09bb23762 + +diff --git a/net/batman-adv/network-coding.c b/net/batman-adv/network-coding.c +index 34caf129a9bf5531360f798be6a7059bad26a50f..7f1be5a287575d51ffd5b4e7ecf540b8fd7de700 100644 +--- a/net/batman-adv/network-coding.c ++++ b/net/batman-adv/network-coding.c +@@ -1021,15 +1021,8 @@ static struct batadv_nc_path *batadv_nc_get_path(struct batadv_priv *bat_priv, + */ + static u8 batadv_nc_random_weight_tq(u8 tq) + { +- u8 rand_val, rand_tq; +- +- get_random_bytes(&rand_val, sizeof(rand_val)); +- + /* randomize the estimated packet loss (max TQ - estimated TQ) */ +- rand_tq = rand_val * (BATADV_TQ_MAX_VALUE - tq); +- +- /* normalize the randomized packet loss */ +- rand_tq /= BATADV_TQ_MAX_VALUE; ++ u8 rand_tq = prandom_u32_max(BATADV_TQ_MAX_VALUE + 1 - tq); + + /* convert to (randomized) estimated tq again */ + return BATADV_TQ_MAX_VALUE - rand_tq; diff --git a/batman-adv/patches/0043-batman-adv-Fix-refcnt-leak-in-batadv_show_throughput.patch b/batman-adv/patches/0043-batman-adv-Fix-refcnt-leak-in-batadv_show_throughput.patch new file mode 100644 index 0000000..f6c45df --- /dev/null +++ b/batman-adv/patches/0043-batman-adv-Fix-refcnt-leak-in-batadv_show_throughput.patch @@ -0,0 +1,38 @@ +From: Xiyu Yang +Date: Wed, 15 Apr 2020 16:31:50 +0800 +Subject: batman-adv: Fix refcnt leak in batadv_show_throughput_override + +batadv_show_throughput_override() invokes batadv_hardif_get_by_netdev(), +which gets a batadv_hard_iface object from net_dev with increased refcnt +and its reference is assigned to a local pointer 'hard_iface'. + +When batadv_show_throughput_override() returns, "hard_iface" becomes +invalid, so the refcount should be decreased to keep refcount balanced. + +The issue happens in the normal path of +batadv_show_throughput_override(), which forgets to decrease the refcnt +increased by batadv_hardif_get_by_netdev() before the function returns, +causing a refcnt leak. + +Fix this issue by calling batadv_hardif_put() before the +batadv_show_throughput_override() returns in the normal path. + +Fixes: c513176e4b7a ("batman-adv: add throughput override attribute to hard_ifaces") +Signed-off-by: Xiyu Yang +Signed-off-by: Xin Tan +Signed-off-by: Sven Eckelmann + +Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/f301bfed59b146a63471d0f147b767d7cafede6f + +diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c +index 09427fc6494a157554d8b19f3481a878a9f97bba..7f7de0b16aa7ab70986735fbd9b42fd02de8a924 100644 +--- a/net/batman-adv/sysfs.c ++++ b/net/batman-adv/sysfs.c +@@ -1126,6 +1126,7 @@ static ssize_t batadv_show_throughput_override(struct kobject *kobj, + + tp_override = atomic_read(&hard_iface->bat_v.throughput_override); + ++ batadv_hardif_put(hard_iface); + return sprintf(buff, "%u.%u MBit\n", tp_override / 10, + tp_override % 10); + } diff --git a/batman-adv/patches/0044-batman-adv-Fix-refcnt-leak-in-batadv_store_throughpu.patch b/batman-adv/patches/0044-batman-adv-Fix-refcnt-leak-in-batadv_store_throughpu.patch new file mode 100644 index 0000000..bf93520 --- /dev/null +++ b/batman-adv/patches/0044-batman-adv-Fix-refcnt-leak-in-batadv_store_throughpu.patch @@ -0,0 +1,39 @@ +From: Xiyu Yang +Date: Wed, 15 Apr 2020 16:35:21 +0800 +Subject: batman-adv: Fix refcnt leak in batadv_store_throughput_override + +batadv_show_throughput_override() invokes batadv_hardif_get_by_netdev(), +which gets a batadv_hard_iface object from net_dev with increased refcnt +and its reference is assigned to a local pointer 'hard_iface'. + +When batadv_store_throughput_override() returns, "hard_iface" becomes +invalid, so the refcount should be decreased to keep refcount balanced. + +The issue happens in one error path of +batadv_store_throughput_override(). When batadv_parse_throughput() +returns NULL, the refcnt increased by batadv_hardif_get_by_netdev() is +not decreased, causing a refcnt leak. + +Fix this issue by jumping to "out" label when batadv_parse_throughput() +returns NULL. + +Fixes: c513176e4b7a ("batman-adv: add throughput override attribute to hard_ifaces") +Signed-off-by: Xiyu Yang +Signed-off-by: Xin Tan +Signed-off-by: Sven Eckelmann + +Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/b69cd8bdbfd6fa7e61878c2fa9e6637406f40dd9 + +diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c +index 7f7de0b16aa7ab70986735fbd9b42fd02de8a924..976b038e53bf934332a39bad8a5509ca1aac0add 100644 +--- a/net/batman-adv/sysfs.c ++++ b/net/batman-adv/sysfs.c +@@ -1093,7 +1093,7 @@ static ssize_t batadv_store_throughput_override(struct kobject *kobj, + ret = batadv_parse_throughput(net_dev, buff, "throughput_override", + &tp_override); + if (!ret) +- return count; ++ goto out; + + old_tp_override = atomic_read(&hard_iface->bat_v.throughput_override); + if (old_tp_override == tp_override) diff --git a/batman-adv/patches/0045-batman-adv-Fix-refcnt-leak-in-batadv_v_ogm_process.patch b/batman-adv/patches/0045-batman-adv-Fix-refcnt-leak-in-batadv_v_ogm_process.patch new file mode 100644 index 0000000..cf16c46 --- /dev/null +++ b/batman-adv/patches/0045-batman-adv-Fix-refcnt-leak-in-batadv_v_ogm_process.patch @@ -0,0 +1,39 @@ +From: Xiyu Yang +Date: Mon, 20 Apr 2020 13:37:20 +0800 +Subject: batman-adv: Fix refcnt leak in batadv_v_ogm_process + +batadv_v_ogm_process() invokes batadv_hardif_neigh_get(), which returns +a reference of the neighbor object to "hardif_neigh" with increased +refcount. + +When batadv_v_ogm_process() returns, "hardif_neigh" becomes invalid, so +the refcount should be decreased to keep refcount balanced. + +The reference counting issue happens in one exception handling paths of +batadv_v_ogm_process(). When batadv_v_ogm_orig_get() fails to get the +orig node and returns NULL, the refcnt increased by +batadv_hardif_neigh_get() is not decreased, causing a refcnt leak. + +Fix this issue by jumping to "out" label when batadv_v_ogm_orig_get() +fails to get the orig node. + +Fixes: 667996ebeab4 ("batman-adv: OGMv2 - implement originators logic") +Signed-off-by: Xiyu Yang +Signed-off-by: Xin Tan +Signed-off-by: Sven Eckelmann + +Origin: upstream, https://git.open-mesh.org/batman-adv.git/commit/4515f5e6a4ccbe1c563b05f2d487eb9eef3c9740 + +diff --git a/net/batman-adv/bat_v_ogm.c b/net/batman-adv/bat_v_ogm.c +index bf9ea404abe7cbe1dd2113881856cd35b718b7d1..0458de53cb64b2da51de492ffa27f33068351cc8 100644 +--- a/net/batman-adv/bat_v_ogm.c ++++ b/net/batman-adv/bat_v_ogm.c +@@ -735,7 +735,7 @@ static void batadv_v_ogm_process(const struct sk_buff *skb, int ogm_offset, + + orig_node = batadv_v_ogm_orig_get(bat_priv, ogm_packet->orig); + if (!orig_node) +- return; ++ goto out; + + neigh_node = batadv_neigh_node_get_or_create(orig_node, if_incoming, + ethhdr->h_source); -- 2.30.2