Melroy van den Berg | 15 Dec 16:12 2014
Picon

Out-of-date batman-adv module in Linux Mint (Ubuntu)

Hi devs,

I found out that the batman-adv kernel modules is quite out-dated in
Linux Mint (and probably also Ubuntu/Debian unstable).

Using 'lsmod batman-adv' it says: version 2013.5.0. But the current
version is 2014.3.0.

Is it on the planning to update to the newest version? Or what could
we / I do to update the package(s)?

Thanks!

Kind regards,
Melroy van den Berg

Linus Lüssing | 13 Dec 23:32 2014

[PATCH maint] batman-adv: fix potential TT client + orig-node memory leak

This patch fixes a potential memory leak which can occur once an
originator times out. On timeout the according global translation table
entry might not get purged correctly. Furthermore, the non purged TT
entry will cause its orig-node to leak, too. Which additionally can lead
to the new multicast optimization feature not kicking in because of a
therefore bogus counter.

In the wild with larger mesh networks we saw this leak quite regularly,
resulting in routers to reboot or killed processes. This was because
of a combination of two bugs: The bug fixed by commit
"batman-adv: fix delayed foreign originator recognition" (8a2ad5204674)
amplified this memory leak heavily. Since that commit I'd expect
it to happen rarely, probably only in paused and resumed VMs and
devices previously in stand-by.

The issue this patch fixes is caused by batadv_orig_node_free_rcu()
never being called because of not yet released references to the
orig-node. References which were supposed to be released through
batadv_orig_node_free_rcu()->batadv_tt_global_del_orig().

Fixing the issue by moving batadv_tt_global_del_orig() out of the rcu
callback.

Signed-off-by: Linus Lüssing <linus.luessing@...>
---
 originator.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/originator.c b/originator.c
index 648bdba..bea8198 100644
(Continue reading)

Marek Lindner | 13 Dec 08:21 2014
Picon

Re: batman-adv: Fix dmesg | grep, use batman_adv (instead of batman-adv)


Hi,

the mail below went to the wrong mailing list (commits <at>  is read-only).

Cheers,
Marek

On Friday 12 December 2014 22:04:10 Melroy van den Berg wrote:
> Hi,
> 
> This is my first patch :). This is small one, but it's a bug.
> 
> From 414638b05aacdfb111c8f4bf0e35d63b65b8ebdf Mon Sep 17 00:00:00 2001
> From: Melroy van den Berg <webmaster1989@...>
> Date: Fri, 12 Dec 2014 21:56:32 +0100
> Subject: [PATCH] batman-adv: Use batman_adv (instead of batman-adv) to grep
> in
>  dmesg, because batman_adv is the real module name.
> 
> Signed-off-by: Melroy van den Berg <webmaster1989@...>
> ---
>  README | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/README b/README
> index 58e4904..4f401e5 100644
> --- a/README
> +++ b/README
>  <at>  <at>  -135,7 +135,7  <at>  <at>  mands: dmesg, logread, or looking in the files
(Continue reading)

Markus Pargmann | 2 Dec 12:16 2014
Picon

[PATCH 00/31] batman-adv: Cleanups

Hi,

this series contains some major cleanup patches (at the beginning of the
series) and some minor cleanups at the end of the series. The behavior of
batman should not be influenced by this series as these patches are only
transformations to make the code more readable and maintainable. If it does
influence behavior something is wrong with this series. The series was tested
on ARM SoC with mwifiex driver.

Major changes (Patch 1-12):
 - Compiling debugfs.c only when CONFIG_DEBUG_FS is selected. This reduces the
   amount of unnecessary code that is executed. At the moment all calls to
   debugfs functions will result in NOOPs. However there is some more code that
   we simply don't need without DEBUG_FS.
 - tvlv is separated from the large main.c file into its own tvlv.c. I don't
   see a reason to have this set of functions for tvlv inside the main.c file.
 - The hashtable implementation now has accessor functions to avoid direct
   access on the private hashtable structure. This should improve the hashtable
   interface and create proper encapsulation.

Minor changes (Patch 13-31):
 - Removing unnecessary return value variables
 - Fixing some comments
 - Reordering functions to increase readability
 - Coding style fixes
 - Declare boolean return types as bool
 - Add missing includes

Best Regards,

(Continue reading)

Sven Eckelmann | 1 Dec 13:59 2014

[PATCH] batman-adv: Unify fragment size calculation

The fragmentation code was replaced in 9b3eab61754d74a93c9840c296013fe3b4a1b606
("batman-adv: Receive fragmented packets and merge") by an implementation which
can handle up to 16 fragments of a packet. The packet is prepared for the split
in fragments by the function batadv_frag_send_packet and the actual split is
done by batadv_frag_create.

Both functions calculate the size of a fragment themself. But their calculation
differs because batadv_frag_send_packet also subtracts ETH_HLEN. Therefore,
the check in batadv_frag_send_packet if a full fragment can be created may
return true even when batadv_frag_create cannot create a full fragment.

The function batadv_frag_create doesn't check the size of the skb before
splitting it and therefore might try to create a larger fragment than the
remaining buffer. This creates an integer underflow and an invalid len is given
to skb_split.

Signed-off-by: Sven Eckelmann <sven@...>
---
 fragmentation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fragmentation.c b/fragmentation.c
index 0ab228f..9e06457 100644
--- a/fragmentation.c
+++ b/fragmentation.c
 <at>  <at>  -433,7 +433,7  <at>  <at>  bool batadv_frag_send_packet(struct sk_buff *skb,
 	 * fragments larger than BATADV_FRAG_MAX_FRAG_SIZE
 	 */
 	mtu = min_t(unsigned, mtu, BATADV_FRAG_MAX_FRAG_SIZE);
-	max_fragment_size = (mtu - header_size - ETH_HLEN);
(Continue reading)

Sven Eckelmann | 1 Dec 10:37 2014

[PATCHv3 1/2] batman-adv: Check total_size when queueing fragments

The fragmentation code was replaced in 9b3eab61754d74a93c9840c296013fe3b4a1b606
("batman-adv: Receive fragmented packets and merge") by an implementation which
handles the queueing+merging of fragments based on their size and the
total_size of the non-fragmented packet. This total_size is announced by each
fragment. The new implementation doesn't check if the the total_size
information of the packets inside one chain is consistent.

This is consistency check is recommended to allow using any of the packets in
the queue to decide whether all fragments of a packet are received or not.

Signed-off-by: Sven Eckelmann <sven@...>
---
This is only compile tested

v3.
 - withdrawing from inclusion in maint
 - remove wrong example from the commit message
v2:
 - proposed for maint
 - changed commit message slightly
---
 fragmentation.c | 7 +++++--
 types.h         | 2 ++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/fragmentation.c b/fragmentation.c
index 5251aa1..6e4c957 100644
--- a/fragmentation.c
+++ b/fragmentation.c
 <at>  <at>  -161,6 +161,7  <at>  <at>  static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node,
(Continue reading)

Sven Eckelmann | 1 Dec 10:32 2014

[PATCH-maint] batman-adv: Calculate extra tail size based on queued fragments

The fragmentation code was replaced in 9b3eab61754d74a93c9840c296013fe3b4a1b606
("batman-adv: Receive fragmented packets and merge"). The new code provided a
mostly unused parameter skb for the merging function. It is used inside the
function to calculate the additionally needed skb tailroom. But instead of
increasing its own tailroom, it is only increasing the tailroom of the first
queued skb. This is not correct in some situations because the first queued
entry can be a different one than the parameter.

An observed problem was:

1. packet with size 104, total_size 1464, fragno 1 was received
   - packet is queued
2. packet with size 1400, total_size 1464, fragno 0 was received
   - packet is queued at the end of the list
3. enough data was received and can be given to the merge function
   (1464 == (1400 - 20) + (104 - 20))
   - merge functions gets 1400 byte large packet as skb argument
4. merge function gets first entry in queue (104 byte)
   - stored as skb_out
5. merge function calculates the required extra tail as total_size - skb->len
   - pskb_expand_head tail of skb_out with 64 bytes
6. merge function tries to squeeze the extra 1380 bytes from the second queued
   skb (1400 byte aka skb parameter) in the 64 extra tail bytes of skb_out

Instead calculate the extra required tail bytes for skb_out also using skb_out
instead of using the parameter skb. The skb parameter is only used to get the
total_size from the last received packet. This is also the total_size used to
decide that all fragments were received.

Signed-off-by: Sven Eckelmann <sven@...>
(Continue reading)

Sven Eckelmann | 1 Dec 08:49 2014

Fragmentation and padding in batman-adv

Hi,

I've just noticed that the padding by the underlying network protocol seems 
not to be handled by the fragmentation. Maybe Martin can correct me. I will 
now use following assumptions:

 * the fragmentation code is sending first the last part of the packet and
   tries to fill the complete skb (max 1400 byte)
 * the mtu of the underlying device is 1400
 * the minimum packet size (user data + eth header) of the underlying device
   is 70
 * the packet send by the user would end up to be 1401 bytes before
   fragmentation

Ok, then I would guess that the fragmentation code would try to generate 
fragments with the max_fragment_size 1366 (+headers of course, not sure why 
the code assumes that the ethernet header is part of the MTU). This would mean 
that the 1401 byte packet is split into a 1366 byte fragment (+header) and a 
35 byte fragment (+header).

But the 35 byte fragment containing the first part of the packet is (even with 
the headers) still smaller than the required packet size of the underlying 
device. Now some extra bytes are added as padding to the last fragment 
(containing the first part of the original packet).

The receiving node cannot merge the fragments anymore because the length of 
the last fragment skb will be too large and therefore the
total_size < chain->size.

Even when it could be merged (because of some bug in the size check) then the 
(Continue reading)

Sven Eckelmann | 1 Dec 00:05 2014

[RFC] batman-adv: Calculate extra tail size based on queued fragments

The fragmentation code was replaced in 9b3eab61754d74a93c9840c296013fe3b4a1b606
("batman-adv: Receive fragmented packets and merge"). The new code provided a
mostly unused parameter skb for the merging function. It is used inside the
function to calculate the additionally needed skb tailroom. But instead of
increasing its own tailroom, it is only increasing the tailroom of the first
queued skb. This is not correct in most situations because the first queued
entry can be a different one than the parameter.

An observed problem was:

1. packet with size 104, total_size 1464, fragno 1 was received
   - packet is queued
2. packet with size 1400, total_size 1464, fragno 0 was received
   - packet is queued at the end of the list
3. enough data was received and can be given to the merge function
   (1464 == (1400 - 20) + (104 - 20))
   - merge functions gets 1400 byte large packet as skb argument
4. merge function gets first entry in queue (104 byte)
   - stored as skb_out
5. merge function calculates the required extra tail as total_size - skb->len
   - pskb_expand_head tail of skb_out with 64 bytes
6. merge function tries to squeeze the extra 1380 bytes from the second queued
   skb (1400 byte aka skb parameter) in the 64 extra tail bytes of skb_out

Instead take only skbs from the queue to merge a packet and remove the
problematic parameter.

Signed-off-by: Sven Eckelmann <sven@...>
Reported-by: Philipp Psurek <philipp.psurek@...>
---
(Continue reading)

Sven Eckelmann | 30 Nov 22:33 2014

[PATCH-maint] batman-adv: Check total_size when reassembling fragments

The fragmentation code was replaced in 9b3eab61754d74a93c9840c296013fe3b4a1b606
("batman-adv: Receive fragmented packets and merge") by an implementation which
handles the queueing+merging of fragments based on their size and the
total_size of the non-fragmented packet. This total_size is announced by each
fragment. The new implementation doesn't check if the the total_size
information of the packets inside one chain is consistent. This allows an
attacker to inject packets belonging to the same fragmentation sequence number
with varying total_size information. The missing validation can cause a crash
when the fragments are merged because the total_size information is only
retrieved from the first packet by batadv_frag_merge_packets. But the queueing
function batadv_frag_insert_packet always uses the total_size from the latest
packet to check if the fragmented packet was transferred completely and is now
ready to be merged.

Assume two packets with the size x and y.

1. first packet (fragno 1) is sent with a size x and the total_size x+y' (y' < y)
2. second packet (fragno 0) is sent with a size y and the total_size x+y

The fragmentation code would try to merge the two packets because the
accumulated packets have a combined size of x+y and the second packet was sent
with total_size of x+y.

The fragments merging code only took the information from the first packet with
the total_size x+y' and created a buffer with enough space for x+y' bytes. But
the second packet cannot be copied inside the prepared free space because it is
y-y' bytes larger than the remaining space.

Signed-off-by: Sven Eckelmann <sven@...>
Acked-by: Martin Hundebøll <martin@...>
(Continue reading)

Markus Pargmann | 29 Nov 19:07 2014
Picon

[PATCH] batman-adv: Kconfig, Add missing DEBUG_FS dependency

BATMAN_ADV_DEBUG is using debugfs files for the debugging log. So it
depends on DEBUG_FS which is missing as dependency in the Kconfig file.

Signed-off-by: Markus Pargmann <mpa@...>
---
 net/batman-adv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/batman-adv/Kconfig b/net/batman-adv/Kconfig
index 11660a3aab5a..c6fc8f756c9a 100644
--- a/net/batman-adv/Kconfig
+++ b/net/batman-adv/Kconfig
 <at>  <at>  -62,6 +62,7  <at>  <at>  config BATMAN_ADV_MCAST
 config BATMAN_ADV_DEBUG
 	bool "B.A.T.M.A.N. debugging"
 	depends on BATMAN_ADV
+	depends on DEBUG_FS
 	help
 	  This is an option for use by developers; most people should
 	  say N here. This enables compilation of support for
--

-- 
2.1.3


Gmane