* [PATCH 1/3] batman-adv: Calculate extra tail size based on queued fragments
2014-12-20 12:48 Stable fixes for batman-adv Sven Eckelmann
@ 2014-12-20 12:48 ` Sven Eckelmann
2014-12-20 12:48 ` [PATCH 2/3] batman-adv: Unify fragment size calculation Sven Eckelmann
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2014-12-20 12:48 UTC (permalink / raw)
To: davem; +Cc: netdev, Sven Eckelmann
The fragmentation code was replaced in 610bfc6bc99bc83680d190ebc69359a05fc7f605
("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.
Reported-by: Philipp Psurek <philipp.psurek@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Acked-by: Martin Hundebøll <martin@hundeboll.net>
---
Problem is in the kernel since v3.13 and may be important for the stable tree.
net/batman-adv/fragmentation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index fc1835c..8af3461 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -251,7 +251,7 @@ batadv_frag_merge_packets(struct hlist_head *chain, struct sk_buff *skb)
kfree(entry);
/* Make room for the rest of the fragments. */
- if (pskb_expand_head(skb_out, 0, size - skb->len, GFP_ATOMIC) < 0) {
+ if (pskb_expand_head(skb_out, 0, size - skb_out->len, GFP_ATOMIC) < 0) {
kfree_skb(skb_out);
skb_out = NULL;
goto free;
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] batman-adv: Unify fragment size calculation
2014-12-20 12:48 Stable fixes for batman-adv Sven Eckelmann
2014-12-20 12:48 ` [PATCH 1/3] batman-adv: Calculate extra tail size based on queued fragments Sven Eckelmann
@ 2014-12-20 12:48 ` Sven Eckelmann
2014-12-20 12:48 ` [PATCH 3/3] batman-adv: avoid NULL dereferences and fix if check Sven Eckelmann
2014-12-21 9:39 ` Stable fixes for batman-adv Antonio Quartulli
3 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2014-12-20 12:48 UTC (permalink / raw)
To: davem; +Cc: netdev, Sven Eckelmann
The fragmentation code was replaced in 610bfc6bc99bc83680d190ebc69359a05fc7f605
("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 "can 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@narfation.org>
---
Problem is in the kernel since v3.13 and may be important for the stable tree.
net/batman-adv/fragmentation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c
index 8af3461..00f9e14 100644
--- a/net/batman-adv/fragmentation.c
+++ b/net/batman-adv/fragmentation.c
@@ -434,7 +434,7 @@ 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);
+ max_fragment_size = mtu - header_size;
max_packet_size = max_fragment_size * BATADV_FRAG_MAX_FRAGMENTS;
/* Don't even try to fragment, if we need more than 16 fragments */
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] batman-adv: avoid NULL dereferences and fix if check
2014-12-20 12:48 Stable fixes for batman-adv Sven Eckelmann
2014-12-20 12:48 ` [PATCH 1/3] batman-adv: Calculate extra tail size based on queued fragments Sven Eckelmann
2014-12-20 12:48 ` [PATCH 2/3] batman-adv: Unify fragment size calculation Sven Eckelmann
@ 2014-12-20 12:48 ` Sven Eckelmann
2014-12-21 9:39 ` Stable fixes for batman-adv Antonio Quartulli
3 siblings, 0 replies; 6+ messages in thread
From: Sven Eckelmann @ 2014-12-20 12:48 UTC (permalink / raw)
To: davem; +Cc: netdev, Antonio Quartulli, Marek Lindner
From: Antonio Quartulli <antonio@meshcoding.com>
Gateway having bandwidth_down equal to zero are not accepted
at all and so never added to the Gateway list.
For this reason checking the bandwidth_down member in
batadv_gw_out_of_range() is useless.
This is probably a copy/paste error and this check was supposed
to be "!gw_node" only. Moreover, the way the check is written
now may also lead to a NULL dereference.
Fix this by rewriting the if-condition properly.
Introduced by 414254e342a0d58144de40c3da777521ebaeeb07
("batman-adv: tvlv - gateway download/upload bandwidth container")
Signed-off-by: Antonio Quartulli <antonio@meshcoding.com>
Reported-by: David Binderman <dcb314@hotmail.com>
Signed-off-by: Marek Lindner <mareklindner@neomailbox.ch>
---
Problem is in the kernel since v3.13 and may be important for the stable tree.
net/batman-adv/gateway_client.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
index 90cff58..e0bcf9e 100644
--- a/net/batman-adv/gateway_client.c
+++ b/net/batman-adv/gateway_client.c
@@ -810,7 +810,7 @@ bool batadv_gw_out_of_range(struct batadv_priv *bat_priv,
goto out;
gw_node = batadv_gw_node_get(bat_priv, orig_dst_node);
- if (!gw_node->bandwidth_down == 0)
+ if (!gw_node)
goto out;
switch (atomic_read(&bat_priv->gw_mode)) {
--
2.1.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: Stable fixes for batman-adv
2014-12-20 12:48 Stable fixes for batman-adv Sven Eckelmann
` (2 preceding siblings ...)
2014-12-20 12:48 ` [PATCH 3/3] batman-adv: avoid NULL dereferences and fix if check Sven Eckelmann
@ 2014-12-21 9:39 ` Antonio Quartulli
2014-12-22 21:14 ` David Miller
3 siblings, 1 reply; 6+ messages in thread
From: Antonio Quartulli @ 2014-12-21 9:39 UTC (permalink / raw)
To: Sven Eckelmann, davem; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 679 bytes --]
Hi Sven,
On 20/12/14 13:48, Sven Eckelmann wrote:
> Hi,
>
> it seems that patches aren't forwarded anymore (since August?) from batman-adv
> to the netdev mailing list. Please correct me if I am wrong.
>
Thank you very much for sending these patches to stable. Unfortunately
in the last months I've been really busy with work and my focus has been
shifted a bit away ... I should be able to get back in sync after these
Christmas and New Year holidays.
David, please merge these fixes and queue them for stable even if this
is not the standard pull request we usually do.
Thank you all again for your work.
Regards,
--
Antonio Quartulli
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread