[B.A.T.M.A.N.] [PATCH] batman-adv: Unify fragment size calculation

[B.A.T.M.A.N.] [PATCH] batman-adv: Unify fragment size calculation

[Sven Eckelmann]

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@narfation.org>
---
 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
@@ -433,7 +433,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.3

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Unify fragment size calculation

[Martin Hundebøll]

Acked-by: Martin Hundebøll <martin@hundeboll.net>

On 2014-12-01 13:59, Sven Eckelmann wrote:
> 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@narfation.org>
> ---
>   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
> @@ -433,7 +433,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 */
>

-- 
Kind Regards,
Martin Hundebøll
Frederiks Allé 99A, 1.th
8000 Aarhus C

+45 61 65 54 61
martin@hundeboll.net

Re: [B.A.T.M.A.N.] [PATCH] batman-adv: Unify fragment size calculation

[Marek Lindner]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

On Monday 01 December 2014 13:59:44 Sven Eckelmann wrote:
> 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@narfation.org>
> ---
>  fragmentation.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied in revision eddbc3d.

Thanks,
Marek

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 473 bytes --]