netdev - suspicius csum initialization in vmxnet3_rx

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>] [day] [month] [year] [list]

Date:   Thu, 24 May 2018 10:47:21 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Ronak Doshi <doshir@...are.com>,
        Shrikrishna Khare <skhare@...are.com>, pv-drivers@...are.com
Cc:     netdev@...r.kernel.org, Neil Horman <nhorman@...driver.com>
Subject: suspicius csum initialization in vmxnet3_rx_csum

Hi all,

we are hitting the BUG() condition in skb_checksum_help() -
skb_checksum_start_offset(skb) >= skb_headlen(skb) for skb received
from vmnxnet3 and queued from OVS to user-space

I think that the root cause is in vmxnet3_rx_csum():

			if (gdesc->rcd.csum) {
                                skb->csum = htons(gdesc->rcd.csum);
                                skb->ip_summed = CHECKSUM_PARTIAL;

CHECKSUM_PARTIAL looks suspicious here, as the csum field is
initialized, instead of csum_offset/csum_start. To be honest I find
also strange that the csum value is converted from host byte order.

I'm wild guessing something like the below patch should fix the issue,
but I'm not familiar with the vmxnet3 code. Can you please have a look?

Thank you,

Paolo
---
diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index 9ebe2a689966..06ade074c32c 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -1172,7 +1172,7 @@ vmxnet3_rx_csum(struct vmxnet3_adapter *adapter,
 		} else {
 			if (gdesc->rcd.csum) {
 				skb->csum = htons(gdesc->rcd.csum);
-				skb->ip_summed = CHECKSUM_PARTIAL;
+				skb->ip_summed = CHECKSUM_COMPLETE;
 			} else {
 				skb_checksum_none_assert(skb);
 			}