NetBSD Problem Report #44070

From track@Plectere.com  Mon Nov  8 22:26:55 2010
Return-Path: <track@Plectere.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 5974C63BAC2
	for <gnats-bugs@gnats.netbsd.org>; Mon,  8 Nov 2010 22:26:55 +0000 (UTC)
Message-Id: <201011082126.oA8LQGfW006863@Plectere.com>
Date: Mon, 8 Nov 2010 13:26:16 -0800 (PST)
From: List Mail User <track@Plectere.com>
To: gnats-bugs@gnats.netbsd.org
Cc: track@Plectere.com
Subject: panic in ip_nat.c when using single port range(s)

>Number:         44070
>Category:       kern
>Synopsis:       ipf outgoing NAT can panic with divide by zero
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 08 22:30:04 +0000 2010
>Closed-Date:    Thu Aug 18 12:10:41 +0000 2016
>Last-Modified:  Thu Aug 18 12:15:00 +0000 2016
>Originator:     track@plectere.com
>Release:        NetBSD 5.99.39
>Organization:

>Environment:


System: NetBSD svcs 5.99.39 NetBSD 5.99.39 (GENERIC) #4: Mon Aug 23 09:47:33 PDT 2010 root@svcs:/usr/obj/sys/arch/i386/compile/GENERIC i386
Architecture: i386
Machine: i386
>Description:

	When port "randomization" was added to IPF's port ranges, one instance
was left with an off-by-one bug, which can/does result in a kernel panic if a
user/admin attempts to "force" redirection to a particular port.

	Also, the existing "bug" prevents the maximum port value (the range
is/was documented as inclusive) from ever being used.

>How-To-Repeat:
	in /etc/ipf.conf
map $some_if blah,blah -> $some_ip blah,blah portmap $proto x:y (with x=y)
>Fix:
	Least invasive fix below & matches other places in the file;  It
could use an "if" test for equality just as well. e.g.:
		port = ntohs(np->in_pmin);
		if (np->in_pmin != np->in_pmax)
			port += ipf_random() % ... ;

Index: ip_nat.c
===================================================================
RCS file: /cvsroot/src/sys/dist/ipf/netinet/ip_nat.c,v
retrieving revision 1.41
diff -u -r1.41 ip_nat.c
--- ip_nat.c	17 Apr 2010 21:00:44 -0000	1.41
+++ ip_nat.c	8 Nov 2010 21:00:19 -0000
@@ -2064,7 +2064,7 @@
 				port = np->in_pnext;
 			} else {
 				port = ipf_random() % (ntohs(np->in_pmax) -
-						       ntohs(np->in_pmin));
+						       ntohs(np->in_pmin) + 1);
 				port += ntohs(np->in_pmin);
 			}
 			port = htons(port);

>Release-Note:

>Audit-Trail:
From: darrenr@NetBSD.org (Darren Reed)
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Subject: Re: kern/44070
Date: Tue,  9 Nov 2010 04:38:28 +0000 (UTC)

 This is already public knowledge (and patch looks right) so no need
 to make this confidential:

   http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=6792026

From: List Mail User <track@Plectere.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: Subject: Re: kern/44070
Date: Tue, 9 Nov 2010 00:06:44 -0800 (PST)

 > This is already public knowledge (and patch looks right) so no need
 > to make this confidential:

 	Confidential!?  I must need more sleep (or more caffeine).

From: List Mail User <track@Plectere.com>
To: gnats-bugs@NetBSD.org
Cc: track@Plectere.com
Subject: Re: Subject: Re: kern/44070
Date: Sat, 12 Feb 2011 07:55:29 -0800 (PST)

 	Hi,

 	I see that IPF seems to be allowing to wither in favor of
 PF and NPF, but couldn't this single line fix for a panic get committed?

 	Thanks,

 	Paul Shupak

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44070 CVS commit: src/sys/dist/ipf/netinet
Date: Sat, 12 Feb 2011 13:14:21 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Sat Feb 12 18:14:21 UTC 2011

 Modified Files:
 	src/sys/dist/ipf/netinet: ip_nat.c

 Log Message:
 PR/44070: Avoid zero divide in modulo operations.


 To generate a diff of this commit:
 cvs rdiff -u -r1.41 -r1.42 src/sys/dist/ipf/netinet/ip_nat.c

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

From: List Mail User <track@Plectere.com>
To: christos@netbsd.org, gnats-bugs@netbsd.org
Cc: track@Plectere.com
Subject: Re: PR/44070 CVS commit: src/sys/dist/ipf/netinet
Date: Sat, 12 Feb 2011 10:48:14 -0800 (PST)

 >...
 > Log Message:
 > PR/44070: Avoid zero divide in modulo operations.
 > 
 > 
 > To generate a diff of this commit:
 > cvs rdiff -u -r1.41 -r1.42 src/sys/dist/ipf/netinet/ip_nat.c
 >...

 	Hi again,

 	Actually, the change in 1.42 is unfortunately not correct:
 The problem is that the syntax for minimum and maximum ports is
 _inclusive_ of the endpoint values, therefore any value between
 OR either if bth of the minimum and maximum port values should be
 possible;  The change in rev 1.42 does fix the original problem _I_
 had (i.e. when minimum == maximum to force a single port to be used),
 but the "fix" in 1.42 has a side effect that the maximum port number
 cannot be used for any specification larger than a single port - i.e.
 a rule like "... 167:168" will only use port 167 and can never use 168
 (this seems the "worst" case to me:  A single "fallback" is allowed for
 rare collisions, but doesn't have any effect/functionality because of
 the logic error).

 	Of well, the panic is gone, so I can safely allow any of my
 local changes to bit-rot, but a (new/related) bug still remains :-(

 	Thanks,

 	Paul Shupak

From: christos@zoulas.com (Christos Zoulas)
To: List Mail User <track@Plectere.com>, gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/44070 CVS commit: src/sys/dist/ipf/netinet
Date: Sat, 12 Feb 2011 16:24:18 -0500

 On Feb 12, 10:48am, track@Plectere.com (List Mail User) wrote:
 -- Subject: Re: PR/44070 CVS commit: src/sys/dist/ipf/netinet

 | 	Hi again,
 | 
 | 	Actually, the change in 1.42 is unfortunately not correct:
 | The problem is that the syntax for minimum and maximum ports is
 | _inclusive_ of the endpoint values, therefore any value between
 | OR either if bth of the minimum and maximum port values should be
 | possible;  The change in rev 1.42 does fix the original problem _I_
 | had (i.e. when minimum == maximum to force a single port to be used),
 | but the "fix" in 1.42 has a side effect that the maximum port number
 | cannot be used for any specification larger than a single port - i.e.
 | a rule like "... 167:168" will only use port 167 and can never use 168
 | (this seems the "worst" case to me:  A single "fallback" is allowed for
 | rare collisions, but doesn't have any effect/functionality because of
 | the logic error).
 | 
 | 	Of well, the panic is gone, so I can safely allow any of my
 | local changes to bit-rot, but a (new/related) bug still remains :-(

 Fixed :-) Neither was the patch though because it was possible to
 crash the kernel.

 christos

From: Patrick Welche <prlw1@cam.ac.uk>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: PR/44070
Date: Thu, 18 Aug 2016 11:49:20 +0100

 It looks as though this can be closed? christos?

State-Changed-From-To: open->closed
State-Changed-By: christos@NetBSD.org
State-Changed-When: Thu, 18 Aug 2016 08:10:41 -0400
State-Changed-Why:
fixed a long time ago


From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org, 
	gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, 
	List Mail User <track@Plectere.com>
Cc: 
Subject: Re: PR/44070
Date: Thu, 18 Aug 2016 08:10:10 -0400

 On Aug 18, 10:50am, prlw1@cam.ac.uk (Patrick Welche) wrote:
 -- Subject: Re: PR/44070

 | The following reply was made to PR kern/44070; it has been noted by GNATS.
 | 
 | From: Patrick Welche <prlw1@cam.ac.uk>
 | To: gnats-bugs@netbsd.org
 | Cc: 
 | Subject: Re: PR/44070
 | Date: Thu, 18 Aug 2016 11:49:20 +0100
 | 
 |  It looks as though this can be closed? christos?
 |  

 Yes. I'll close it.

 christos

>Unformatted:
 To: gnats-bugs@gnats.NetBSD.org
 Subject: panic in ip_nat.c when using single port range(s)
 From: track@plectere.com
 Reply-To: track@plectere.com
 X-send-pr-version: 3.95

Home	PR Database Search