NetBSD Problem Report #54443

From ef@math.uni-bonn.de  Tue Aug  6 15:37:06 2019
Return-Path: <ef@math.uni-bonn.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id D24447A170
	for <gnats-bugs@gnats.NetBSD.org>; Tue,  6 Aug 2019 15:37:06 +0000 (UTC)
Message-Id: <20190806153702.DDD3A1E335@trave.math.uni-bonn.de>
Date: Tue,  6 Aug 2019 17:37:02 +0200 (CEST)
From: ef@math.uni-bonn.de
Reply-To: ef@math.uni-bonn.de
To: gnats-bugs@NetBSD.org
Subject: IPFilter and UDP checksum field 0xffff
X-Send-Pr-Version: 3.95

>Number:         54443
>Category:       kern
>Synopsis:       ipf mistakenly regards UDP packet with checksum field 0xffff as bad
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Aug 06 15:40:00 +0000 2019
>Closed-Date:    Sun Aug 18 07:07:54 +0000 2019
>Last-Modified:  Sun Aug 18 07:07:54 +0000 2019
>Originator:     Edgar Fuß
>Release:        NetBSD 8.1_STABLE
>Organization:
Mathematisches Institut der Universität Bonn
>Environment:

>Description:
	With UDP, a checksum value of 0x0000 is represented as 0xffff in the UDP header's checksum field because a field value of 0x0000 means not to check the checksum.
	When verifying the UDP checksum by computing the checksum on the relevant data including the received checksum field (as the NetBSD TCP/IP stack does), 0xffff doesn't need to be special-cased (as 0x0000 does), because due to idiosyncracies of the one's complement arithmetic used to compute the Internet Checksum, 0x0000 and 0xffff mostly (the exception is all other data bytes being zero, which doesn't appear as a packet) mean the same thing being two representations of Zero.
	But IPFilter verifies the UDP checksum by re-computing it on the received packet without the checksum actually received and then comparing the values. In this case, a UDP checksum field value of 0xffff in the received packet /does/ need to be special-cased to match a re-computed Internet Checksum of 0x0000.

	This can cause the NFS client (and thus parts of VFS) to seemingly randomly lock up when a (UDP) NFS reply is continuously (nothing changes in the request, so nothing changes in the reply) dropped.
>How-To-Repeat:
	Enable IPFilter, try to send/receive UDP traffic with a UDP checksum field of 0xffff.
>Fix:
	Index: sys/external/bsd/ipf/netinet/fil.c
	===================================================================
	RCS file: /cvsroot/src/sys/external/bsd/ipf/netinet/fil.c,v
	retrieving revision 1.20.4.2
	diff -u -p -r1.20.4.2 fil.c
	--- sys/external/bsd/ipf/netinet/fil.c	26 Dec 2018 13:18:53 -0000	1.20.4.2
	+++ sys/external/bsd/ipf/netinet/fil.c	6 Aug 2019 15:14:14 -0000
	@@ -6527,8 +6527,11 @@ ipf_checkl4sum(fr_info_t *fin)
			/*NOTREACHED*/
		}

	-	if (csump != NULL)
	+	if (csump != NULL) {
			hdrsum = *csump;
	+		if (fin->fin_p == IPPROTO_UDP && hdrsum == 0xffff)
	+			hdrsum = 0x0000;
	+	}

		if (dosum) {
			sum = fr_cksum(fin, fin->fin_ip, fin->fin_p, fin->fin_dp);

	Or equivalently (sans the DTrace probe) map sum == 0x0000 to 0xffff in the IPPROTO_UDP case.

	The fix hould be up-streamed I guess.

>Release-Note:

>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54443 CVS commit: src/sys/external/bsd/ipf/netinet
Date: Thu, 8 Aug 2019 10:38:53 -0400

 Module Name:	src
 Committed By:	christos
 Date:		Thu Aug  8 14:38:53 UTC 2019

 Modified Files:
 	src/sys/external/bsd/ipf/netinet: fil.c

 Log Message:
 PR/54443: Edgar Fuß: ip mistakenly regards UDP packet with checksum field
 0xffff as bad


 To generate a diff of this commit:
 cvs rdiff -u -r1.29 -r1.30 src/sys/external/bsd/ipf/netinet/fil.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54443 CVS commit: [netbsd-9] src/sys/external/bsd/ipf/netinet
Date: Fri, 9 Aug 2019 19:18:47 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Aug  9 19:18:47 UTC 2019

 Modified Files:
 	src/sys/external/bsd/ipf/netinet [netbsd-9]: fil.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #40):

 	sys/external/bsd/ipf/netinet/fil.c: revision 1.30

 PR/54443: Edgar Fu�: ipf mistakenly regards UDP packet with checksum field
 0xffff as bad


 To generate a diff of this commit:
 cvs rdiff -u -r1.29 -r1.29.2.1 src/sys/external/bsd/ipf/netinet/fil.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54443 CVS commit: [netbsd-8] src/sys/external/bsd/ipf/netinet
Date: Fri, 9 Aug 2019 19:21:05 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Aug  9 19:21:05 UTC 2019

 Modified Files:
 	src/sys/external/bsd/ipf/netinet [netbsd-8]: fil.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #1331):

 	sys/external/bsd/ipf/netinet/fil.c: revision 1.30

 PR/54443: Edgar Fu�: ipf mistakenly regards UDP packet with checksum field
 0xffff as bad


 To generate a diff of this commit:
 cvs rdiff -u -r1.20.4.2 -r1.20.4.3 src/sys/external/bsd/ipf/netinet/fil.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/54443 CVS commit: [netbsd-7] src/sys/external/bsd/ipf/netinet
Date: Fri, 9 Aug 2019 19:24:22 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Fri Aug  9 19:24:22 UTC 2019

 Modified Files:
 	src/sys/external/bsd/ipf/netinet [netbsd-7]: fil.c

 Log Message:
 Pull up following revision(s) (requested by christos in ticket #1701):

 	sys/external/bsd/ipf/netinet/fil.c: revision 1.30

 PR/54443: Edgar Fu�: ipf mistakenly regards UDP packet with checksum field
 0xffff as bad


 To generate a diff of this commit:
 cvs rdiff -u -r1.15.2.3 -r1.15.2.4 src/sys/external/bsd/ipf/netinet/fil.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sun, 18 Aug 2019 07:07:54 +0000
State-Changed-Why:
Applied & pulled up. Thanks for the patch!


>Unformatted:

Home	PR Database Search