Hi David,
here are two bugfixes which we would like to see integrated into net.
Please pull or let me know of any problem!
Thank you, Simon
The following changes since commit 4ea33ef0f9e95b69db9131d7afd98563713e81b0:
batman-adv: Decrease hardif refcnt on fragmentation send error (2017-01-04 08:22:04 +0100)
are available in the git repository at:
git://git.open-mesh.org/linux-merge.git tags/batadv-net-for-davem-20170301
for you to fetch changes up to 51c6b429c0c95e67edd1cb0b548c5cf6a6604763:
batman-adv: Fix transmission of final, 16th fragment (2017-02-21 18:33:35 +0100)
---------------------------------------------------------------- Here are two batman-adv bugfixes:
- fix a potential double free when fragment merges fail, by Sven Eckelmann
- fix failing tranmission of the 16th (last) fragment if that exists, by Linus Lüssing
---------------------------------------------------------------- Linus Lüssing (1): batman-adv: Fix transmission of final, 16th fragment
Sven Eckelmann (1): batman-adv: Fix double free during fragment merge error
net/batman-adv/fragmentation.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
From: Sven Eckelmann sven@narfation.org
The function batadv_frag_skb_buffer was supposed not to consume the skbuff on errors. This was followed in the helper function batadv_frag_insert_packet when the skb would potentially be inserted in the fragment queue. But it could happen that the next helper function batadv_frag_merge_packets would try to merge the fragments and fail. This results in a kfree_skb of all the enqueued fragments (including the just inserted one). batadv_recv_frag_packet would detect the error in batadv_frag_skb_buffer and try to free the skb again.
The behavior of batadv_frag_skb_buffer (and its helper batadv_frag_insert_packet) must therefore be changed to always consume the skbuff to have a common behavior and avoid the double kfree_skb.
Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge") Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/fragmentation.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 0854ebd8613e..31e97e9aee0d 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -239,8 +239,10 @@ static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node, spin_unlock_bh(&chain->lock);
err: - if (!ret) + if (!ret) { kfree(frag_entry_new); + kfree_skb(skb); + }
return ret; } @@ -313,7 +315,7 @@ batadv_frag_merge_packets(struct hlist_head *chain) * * There are three possible outcomes: 1) Packet is merged: Return true and * set *skb to merged packet; 2) Packet is buffered: Return true and set *skb - * to NULL; 3) Error: Return false and leave skb as is. + * to NULL; 3) Error: Return false and free skb. * * Return: true when packet is merged or buffered, false when skb is not not * used. @@ -338,9 +340,9 @@ bool batadv_frag_skb_buffer(struct sk_buff **skb, goto out_err;
out: - *skb = skb_out; ret = true; out_err: + *skb = skb_out; return ret; }
From: Linus Lüssing linus.luessing@c0d3.blue
Trying to split and transmit a unicast packet in 16 parts will fail for the final fragment: After having sent the 15th one with a frag_packet.no index of 14, we will increase the the index to 15 - and return with an error code immediately, even though one more fragment is due for transmission and allowed.
Fixing this issue by moving the check before incrementing the index.
While at it, adding an unlikely(), because the check is actually more of an assertion.
Fixes: ee75ed88879a ("batman-adv: Fragment and send skbs larger than mtu") Signed-off-by: Linus Lüssing linus.luessing@c0d3.blue Signed-off-by: Sven Eckelmann sven@narfation.org Signed-off-by: Simon Wunderlich sw@simonwunderlich.de --- net/batman-adv/fragmentation.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 31e97e9aee0d..11149e5be4e0 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -501,6 +501,12 @@ int batadv_frag_send_packet(struct sk_buff *skb,
/* Eat and send fragments from the tail of skb */ while (skb->len > max_fragment_size) { + /* The initial check in this function should cover this case */ + if (unlikely(frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1)) { + ret = -EINVAL; + goto put_primary_if; + } + skb_fragment = batadv_frag_create(skb, &frag_header, mtu); if (!skb_fragment) { ret = -ENOMEM; @@ -517,12 +523,6 @@ int batadv_frag_send_packet(struct sk_buff *skb, }
frag_header.no++; - - /* The initial check in this function should cover this case */ - if (frag_header.no == BATADV_FRAG_MAX_FRAGMENTS - 1) { - ret = -EINVAL; - goto put_primary_if; - } }
/* Make room for the fragment header. */
From: Simon Wunderlich sw@simonwunderlich.de Date: Wed, 1 Mar 2017 16:53:31 +0100
here are two bugfixes which we would like to see integrated into net.
Please pull or let me know of any problem!
Pulled, thanks Simon.
b.a.t.m.a.n@lists.open-mesh.org
-
David Miller -
Simon Wunderlich