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@narfation.org>
---
 fragmentation.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

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 */
>
  

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