NetBSD Problem Report #46265

From www@NetBSD.org  Tue Mar 27 08:57:56 2012
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	by www.NetBSD.org (Postfix) with ESMTP id E251363C4B1
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 27 Mar 2012 08:57:55 +0000 (UTC)
Message-Id: <20120327085755.213E163BBEC@www.NetBSD.org>
Date: Tue, 27 Mar 2012 08:57:55 +0000 (UTC)
From: xiaosuo@gmail.com
Reply-To: xiaosuo@gmail.com
To: gnats-bugs@NetBSD.org
Subject: NPF: update the tstate->nst_end wrongly
X-Send-Pr-Version: www-1.0

>Number:         46265
>Category:       kern
>Synopsis:       NPF: update the tstate->nst_end wrongly
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    rmind
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Mar 27 09:00:01 +0000 2012
>Closed-Date:    Tue Apr 03 22:19:22 +0000 2012
>Last-Modified:  Tue Jun 26 00:10:02 +0000 2012
>Originator:     Changli Gao
>Release:        CVS/TRUNK
>Organization:
China
>Environment:
I am not using netbsd, but a code reader.
>Description:
file: npf_state_tcp.c

427          * Packet has been passed.
428          *
429          * Negative ackskew might be due to fragmented packets.  Since the
430          * total length of the packet is unknown - bump the boundary.
431          */
432         if (ackskew < 0) {
433                 tstate->nst_end = end;

The above line is incorrect. After reading the corresponding paper again, I think "end" should be "ack". I suppose it is just a typo. :)

434         }
435         /* Keep track of the maximum window seen. */
436         if (fstate->nst_maxwin < win) {
437                 fstate->nst_maxwin = win;
438         }
439         if (SEQ_GT(end, fstate->nst_end)) {
440                 fstate->nst_end = end;
441         }

>How-To-Repeat:
Read the source code carefully.
>Fix:
Replace "end" with "ack".

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: kern-bug-people->rmind
Responsible-Changed-By: rmind@NetBSD.org
Responsible-Changed-When: Mon, 02 Apr 2012 01:00:48 +0000
Responsible-Changed-Why:
Take.


From: "Mindaugas Rasiukevicius" <rmind@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46265 CVS commit: src/sys/net/npf
Date: Tue, 3 Apr 2012 22:14:13 +0000

 Module Name:	src
 Committed By:	rmind
 Date:		Tue Apr  3 22:14:13 UTC 2012

 Modified Files:
 	src/sys/net/npf: npf_state_tcp.c

 Log Message:
 npf_tcp_inwindow: in a case of negative skew, bump the maximum seen value of
 SEQ+LEN in the receiver's side correctly (using ACK from the sender's side).

 PR/46265 from Changli Gao.


 To generate a diff of this commit:
 cvs rdiff -u -r1.3 -r1.4 src/sys/net/npf/npf_state_tcp.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: rmind@NetBSD.org
State-Changed-When: Tue, 03 Apr 2012 22:19:22 +0000
State-Changed-Why:
Good catch, thanks!  Code review is much appreciated.


From: Bernd Ernesti <netbsd@lists.veego.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/46265 (NPF: update the tstate->nst_end wrongly)
Date: Fri, 6 Apr 2012 17:14:53 +0200

 On Tue, Apr 03, 2012 at 10:19:23PM +0000, rmind@NetBSD.org wrote:
 > Synopsis: NPF: update the tstate->nst_end wrongly
 > 
 > State-Changed-From-To: open->closed
 > State-Changed-By: rmind@NetBSD.org
 > State-Changed-When: Tue, 03 Apr 2012 22:19:22 +0000
 > State-Changed-Why:
 > Good catch, thanks!  Code review is much appreciated.

 Should this be pulled up to netbsd-6 too?

 Bernd

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/46265 (NPF: update the tstate->nst_end wrongly)
Date: Fri, 13 Apr 2012 01:09:38 +0000

 On Fri, Apr 06, 2012 at 03:15:04PM +0000, Bernd Ernesti wrote:
  > From: Bernd Ernesti <netbsd@lists.veego.de>
  > To: gnats-bugs@NetBSD.org
  > Cc: 
  > Subject: Re: kern/46265 (NPF: update the tstate->nst_end wrongly)
  > Date: Fri, 6 Apr 2012 17:14:53 +0200
  > 
  >  On Tue, Apr 03, 2012 at 10:19:23PM +0000, rmind@NetBSD.org wrote:
  >  > Synopsis: NPF: update the tstate->nst_end wrongly
  >  > 
  >  > State-Changed-From-To: open->closed
  >  > State-Changed-By: rmind@NetBSD.org
  >  > State-Changed-When: Tue, 03 Apr 2012 22:19:22 +0000
  >  > State-Changed-Why:
  >  > Good catch, thanks!  Code review is much appreciated.
  >  
  >  Should this be pulled up to netbsd-6 too?

 Yes, it seems like it should be.

 -- 
 David A. Holland
 dholland@netbsd.org

From: "Jeff Rizzo" <riz@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/46265 CVS commit: [netbsd-6] src
Date: Tue, 26 Jun 2012 00:07:22 +0000

 Module Name:	src
 Committed By:	riz
 Date:		Tue Jun 26 00:07:21 UTC 2012

 Modified Files:
 	src/sys/net/npf [netbsd-6]: npf.h npf_ctl.c npf_handler.c npf_impl.h
 	    npf_mbuf.c npf_ncode.h npf_sendpkt.c npf_state.c npf_state_tcp.c
 	src/sys/rump/dev/lib/libnpf [netbsd-6]: Makefile
 	src/sys/rump/net/lib/libnet [netbsd-6]: Makefile
 	src/usr.sbin/npf/npfctl [netbsd-6]: Makefile npf.conf.5 npf_build.c
 	    npf_data.c npf_disassemble.c npf_ncgen.c npf_parse.y npf_scan.l
 	    npf_var.c npf_var.h npfctl.c npfctl.h
 Added Files:
 	src/usr.sbin/npf/npftest [netbsd-6]: Makefile npfstream.c npftest.c
 	    npftest.h
 	src/usr.sbin/npf/npftest/libnpftest [netbsd-6]: Makefile
 	    npf_mbuf_subr.c npf_nbuf_test.c npf_processor_test.c
 	    npf_state_test.c npf_table_test.c npf_test.h npf_test_subr.c

 Log Message:
 Pull up following revision(s) (requested by rmind in ticket #354):
 	sys/net/npf/npf_state_tcp.c: revision 1.4
 	sys/net/npf/npf_state_tcp.c: revision 1.5
 	sys/net/npf/npf_state_tcp.c: revision 1.6
 	usr.sbin/npf/npftest/npftest.c: revision 1.1
 	usr.sbin/npf/npftest/libnpftest/npf_mbuf_subr.c: revision 1.1
 	usr.sbin/npf/npftest/npftest.c: revision 1.2
 	usr.sbin/npf/npftest/libnpftest/npf_mbuf_subr.c: revision 1.2
 	usr.sbin/npf/npfctl/npf_data.c: revision 1.11
 	usr.sbin/npf/npftest/npftest.c: revision 1.3
 	usr.sbin/npf/npfctl/npf_data.c: revision 1.12
 	usr.sbin/npf/npftest/npftest.h: revision 1.1
 	usr.sbin/npf/npfctl/npf_parse.y: revision 1.5
 	usr.sbin/npf/npfctl/npf_data.c: revision 1.13
 	sys/net/npf/npf.h: revision 1.16
 	usr.sbin/npf/npftest/npftest.h: revision 1.2
 	usr.sbin/npf/npfctl/npf_parse.y: revision 1.6
 	usr.sbin/npf/npftest/npftest.h: revision 1.3
 	usr.sbin/npf/npfctl/npf_parse.y: revision 1.7
 	usr.sbin/npf/npfctl/npf_ncgen.c: revision 1.10
 	usr.sbin/npf/npfctl/npf_build.c: revision 1.6
 	usr.sbin/npf/npfctl/npf_parse.y: revision 1.8
 	usr.sbin/npf/npfctl/npf_build.c: revision 1.7
 	usr.sbin/npf/npftest/libnpftest/npf_state_test.c: revision 1.1
 	usr.sbin/npf/npftest/libnpftest/npf_nbuf_test.c: revision 1.1
 	usr.sbin/npf/npfctl/npf_build.c: revision 1.8
 	usr.sbin/npf/npftest/libnpftest/npf_table_test.c: revision 1.1
 	usr.sbin/npf/npfctl/npf_build.c: revision 1.9
 	usr.sbin/npf/npfctl/npf.conf.5: revision 1.10
 	usr.sbin/npf/npfctl/npf.conf.5: revision 1.11
 	usr.sbin/npf/npfctl/npf.conf.5: revision 1.12
 	sys/net/npf/npf_state.c: revision 1.7
 	usr.sbin/npf/npfctl/npfctl.c: revision 1.11
 	usr.sbin/npf/npfctl/npfctl.c: revision 1.12
 	usr.sbin/npf/npfctl/Makefile: revision 1.7
 	sys/rump/net/lib/libnet/Makefile: revision 1.14
 	sys/net/npf/npf_mbuf.c: revision 1.7
 	usr.sbin/npf/npftest/Makefile: revision 1.1
 	usr.sbin/npf/npftest/Makefile: revision 1.2
 	usr.sbin/npf/npftest/libnpftest/Makefile: revision 1.1
 	usr.sbin/npf/npfctl/npf_scan.l: revision 1.2
 	usr.sbin/npf/npftest/npfstream.c: revision 1.1
 	usr.sbin/npf/npftest/libnpftest/Makefile: revision 1.2
 	usr.sbin/npf/npfctl/npf_scan.l: revision 1.3
 	usr.sbin/npf/npftest/libnpftest/Makefile: revision 1.3
 	usr.sbin/npf/npfctl/npfctl.h: revision 1.12
 	sys/rump/dev/lib/libnpf/Makefile: revision 1.2
 	usr.sbin/npf/npfctl/npfctl.h: revision 1.14
 	sys/rump/dev/lib/libnpf/Makefile: revision 1.3
 	usr.sbin/npf/npfctl/npfctl.h: revision 1.15
 	usr.sbin/npf/npfctl/npf_ncgen.c: revision 1.9
 	sys/net/npf/npf_ctl.c: revision 1.15
 	usr.sbin/npf/npfctl/npf_var.c: revision 1.4
 	usr.sbin/npf/npfctl/npf_var.h: revision 1.2
 	usr.sbin/npf/npfctl/npf_var.c: revision 1.5
 	sys/net/npf/npf_impl.h: revision 1.13
 	sys/net/npf/npf_sendpkt.c: revision 1.10
 	sys/net/npf/npf_impl.h: revision 1.14
 	usr.sbin/npf/npfctl/npf_disassemble.c: revision 1.4
 	sys/net/npf/npf_impl.h: revision 1.15
 	sys/net/npf/npf_handler.c: revision 1.16
 	usr.sbin/npf/npftest/libnpftest/npf_test.h: revision 1.1
 	usr.sbin/npf/npftest/libnpftest/npf_processor_test.c: revision 1.1
 	usr.sbin/npf/npfctl/npf_disassemble.c: revision 1.5
 	sys/net/npf/npf_handler.c: revision 1.17
 	usr.sbin/npf/npftest/libnpftest/npf_test.h: revision 1.2
 	sys/net/npf/npf_ncode.h: revision 1.7
 	usr.sbin/npf/npftest/libnpftest/npf_test_subr.c: revision 1.1
 	usr.sbin/npf/npftest/libnpftest/npf_test.h: revision 1.3
 	sys/net/npf/npf_ncode.h: revision 1.8
 npf_tcp_inwindow: in a case of negative skew, bump the maximum seen value of
 SEQ+LEN in the receiver's side correctly (using ACK from the sender's side).
 PR/46265 from Changli Gao.
 rumpnet_net: add pfil.c
 Update rumpdev_npf; use WARNS=4.
 Add initial NPF regression tests integrated with RUMP framework (running the
 kernel part of NPF in userland).  Other tests will be added once converted to
 RUMP framework.  All tests are in the public domain.
 Some Makefile fixes from christos@.
 - Fix double-free case on ICMP return case.
 - npf_pfil_register: handle kernels without INET6 option correctly.
 - Reduce some #ifdefs.
 npfctl(8): add show-config command.  Also, update syntax.
 npftest: add a stream processor, which prints out the TCP state information.
 A tool for debugging connection tracking from tcpdump -w captured data.
 npftest: add a module for TCP state tracking and add few test cases.
 npf_state_tcp: add an assert; fix some comments while here.
 - Rework NPF NAT syntax to be more structured and support future additions
   of different types and configurations of NAT.
 - npfctl: improve disassemble and show-config command functionality.
 - Fix custom ICMP code and type filtering.
 make this compile again.
 remove error(1) output
 Remove superfluous Pp
 - make each element of a variable hold a type
 - change get_type to take an index, so we can get the individual types of
   each element (since primitive elements can be in lists)
 - make port_range primitive
 - add a routine to convert a variable of primitives to a variable containing
 - only port ranges.
 remove extra rule that got merged...


 To generate a diff of this commit:
 cvs rdiff -u -r1.14.2.1 -r1.14.2.2 src/sys/net/npf/npf.h
 cvs rdiff -u -r1.12.2.1 -r1.12.2.2 src/sys/net/npf/npf_ctl.c
 cvs rdiff -u -r1.13.2.1 -r1.13.2.2 src/sys/net/npf/npf_handler.c
 cvs rdiff -u -r1.10.2.1 -r1.10.2.2 src/sys/net/npf/npf_impl.h
 cvs rdiff -u -r1.6 -r1.6.14.1 src/sys/net/npf/npf_mbuf.c
 cvs rdiff -u -r1.5.6.1 -r1.5.6.2 src/sys/net/npf/npf_ncode.h
 cvs rdiff -u -r1.8.4.1 -r1.8.4.2 src/sys/net/npf/npf_sendpkt.c
 cvs rdiff -u -r1.6 -r1.6.4.1 src/sys/net/npf/npf_state.c
 cvs rdiff -u -r1.3 -r1.3.2.1 src/sys/net/npf/npf_state_tcp.c
 cvs rdiff -u -r1.1 -r1.1.18.1 src/sys/rump/dev/lib/libnpf/Makefile
 cvs rdiff -u -r1.13 -r1.13.8.1 src/sys/rump/net/lib/libnet/Makefile
 cvs rdiff -u -r1.5.2.1 -r1.5.2.2 src/usr.sbin/npf/npfctl/Makefile
 cvs rdiff -u -r1.9 -r1.9.2.1 src/usr.sbin/npf/npfctl/npf.conf.5
 cvs rdiff -u -r1.4.2.1 -r1.4.2.2 src/usr.sbin/npf/npfctl/npf_build.c
 cvs rdiff -u -r1.10 -r1.10.2.1 src/usr.sbin/npf/npfctl/npf_data.c \
     src/usr.sbin/npf/npfctl/npfctl.c
 cvs rdiff -u -r1.3.2.2 -r1.3.2.3 src/usr.sbin/npf/npfctl/npf_disassemble.c
 cvs rdiff -u -r1.7.2.1 -r1.7.2.2 src/usr.sbin/npf/npfctl/npf_ncgen.c
 cvs rdiff -u -r1.3.2.1 -r1.3.2.2 src/usr.sbin/npf/npfctl/npf_parse.y
 cvs rdiff -u -r1.1 -r1.1.2.1 src/usr.sbin/npf/npfctl/npf_scan.l \
     src/usr.sbin/npf/npfctl/npf_var.h
 cvs rdiff -u -r1.3 -r1.3.2.1 src/usr.sbin/npf/npfctl/npf_var.c
 cvs rdiff -u -r1.11.2.1 -r1.11.2.2 src/usr.sbin/npf/npfctl/npfctl.h
 cvs rdiff -u -r0 -r1.2.2.2 src/usr.sbin/npf/npftest/Makefile
 cvs rdiff -u -r0 -r1.1.2.2 src/usr.sbin/npf/npftest/npfstream.c
 cvs rdiff -u -r0 -r1.3.2.2 src/usr.sbin/npf/npftest/npftest.c \
     src/usr.sbin/npf/npftest/npftest.h
 cvs rdiff -u -r0 -r1.3.2.2 src/usr.sbin/npf/npftest/libnpftest/Makefile \
     src/usr.sbin/npf/npftest/libnpftest/npf_test.h
 cvs rdiff -u -r0 -r1.2.2.2 \
     src/usr.sbin/npf/npftest/libnpftest/npf_mbuf_subr.c \
     src/usr.sbin/npf/npftest/libnpftest/npf_table_test.c
 cvs rdiff -u -r0 -r1.1.4.2 \
     src/usr.sbin/npf/npftest/libnpftest/npf_nbuf_test.c \
     src/usr.sbin/npf/npftest/libnpftest/npf_processor_test.c
 cvs rdiff -u -r0 -r1.1.2.2 \
     src/usr.sbin/npf/npftest/libnpftest/npf_state_test.c \
     src/usr.sbin/npf/npftest/libnpftest/npf_test_subr.c

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

>Unformatted:

Home	PR Database Search