Re: [PATCH] tap: fix uses of l3_len in tap4_handler()

[David Gibson <david@gibson.dropbear.id.au>]

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

On Tue, Aug 29, 2023 at 09:44:06PM +0500, Stas Sergeev wrote:
> l3_len was calculated from the ethernet frame size, and it
> was assumed to be equal to the length stored in an IP packet.
> But if the ethernet frame is padded, then l3_len calculated
> that way can only be used as a bound check to validate the
> length stored in an IP header. It should not be used for
> calculating the l4_len.
> 
> This patch makes sure the small padded ethernet frames are
> properly processed, by trusting the length stored in an IP
> header.
> 
> Signed-off-by: Stas Sergeev <stsp2@yandex.ru>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> CC: Stefano Brivio <sbrivio@redhat.com>
> ---
>  tap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tap.c b/tap.c
> index ee79be0..8d7859c 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -615,7 +615,7 @@ resume:
>  			continue;
>  
>  		hlen = iph->ihl * 4UL;
> -		if (hlen < sizeof(*iph) || htons(iph->tot_len) != l3_len ||
> +		if (hlen < sizeof(*iph) || htons(iph->tot_len) > l3_len ||
>  		    hlen > l3_len)
>  			continue;
>  
> @@ -623,7 +623,7 @@ resume:
>  		if (tap4_is_fragment(iph, now))
>  			continue;
>  
> -		l4_len = l3_len - hlen;
> +		l4_len = htons(iph->tot_len) - hlen;
>  
>  		if (iph->saddr && c->ip4.addr_seen.s_addr != iph->saddr)
>  			c->ip4.addr_seen.s_addr = iph->saddr;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]