netdev - Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for coalesced packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190223084624.0d0c60a0@shemminger-XPS-13-9360>
Date:   Sat, 23 Feb 2019 08:46:24 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Haiyang Zhang <haiyangz@...uxonhyperv.com>
Cc:     haiyangz@...rosoft.com, sashal@...nel.org,
        linux-hyperv@...r.kernel.org, kys@...rosoft.com,
        sthemmin@...rosoft.com, olaf@...fle.de, vkuznets@...hat.com,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH hyperv-fixes] hv_netvsc: Fix IP header checksum for
 coalesced packets

On Fri, 22 Feb 2019 18:25:03 +0000
Haiyang Zhang <haiyangz@...uxonhyperv.com> wrote:

> From: Haiyang Zhang <haiyangz@...rosoft.com>
> 
> Incoming packets may have IP header checksum verified by the host.
> They may not have IP header checksum computed after coalescing.
> This patch re-compute the checksum when necessary, otherwise the
> packets may be dropped, because Linux network stack always checks it.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> ---
>  drivers/net/hyperv/netvsc_drv.c | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 256adbd044f5..cf4897043e83 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -744,6 +744,14 @@ void netvsc_linkstatus_callback(struct net_device *net,
>  	schedule_delayed_work(&ndev_ctx->dwork, 0);
>  }
>  
> +static void netvsc_comp_ipcsum(struct sk_buff *skb)
> +{
> +	struct iphdr *iph = (struct iphdr *)skb->data;

Can you use iphdr(skb) here?

> +
> +	iph->check = 0;
> +	iph->check = ip_fast_csum(iph, iph->ihl);
> +}
> +
>  static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
>  					     struct netvsc_channel *nvchan)
>  {
> @@ -770,9 +778,17 @@ static struct sk_buff *netvsc_alloc_recv_skb(struct net_device *net,
>  	/* skb is already created with CHECKSUM_NONE */
>  	skb_checksum_none_assert(skb);
>  
> -	/*
> -	 * In Linux, the IP checksum is always checked.
> -	 * Do L4 checksum offload if enabled and present.
> +	/* Incoming packets may have IP header checksum verified by the host.
> +	 * They may not have IP header checksum computed after coalescing.
> +	 * We compute it here if the flags are set, because on Linux, the IP
> +	 * checksum is always checked.
> +	 */
> +	if (csum_info && csum_info->receive.ip_checksum_value_invalid &&
> +	    csum_info->receive.ip_checksum_succeeded &&
> +	    skb->protocol == htons(ETH_P_IP))
> +		netvsc_comp_ipcsum(skb);

Does this still handle for coalesced and non-coalesced packets
which are received with bad IP checksum?  My concern is that you are
potentially correcting the checksum for a packet whose received checksum was bad.


> +	/* Do L4 checksum offload if enabled and present.
>  	 */
>  	if (csum_info && (net->features & NETIF_F_RXCSUM)) {
>  		if (csum_info->receive.tcp_checksum_succeeded ||