NetBSD Problem Report #45929

From darrenr@NetBSD.org  Sun Feb  5 13:54:28 2012
Return-Path: <darrenr@NetBSD.org>
Received: by www.NetBSD.org (Postfix, from userid 1110)
	id C0F7663D7A7; Sun,  5 Feb 2012 13:54:28 +0000 (UTC)
Message-Id: <20120205135428.C0F7663D7A7@www.NetBSD.org>
Date: Sun,  5 Feb 2012 13:54:28 +0000 (UTC)
From: darrenr@NetBSD.org
Reply-To: darrenr@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: ipnat does not remove rules with -r
X-Send-Pr-Version: 3.95

>Number:         45929
>Category:       kern
>Synopsis:       ipnat does not remove rules with -r
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Feb 05 13:55:00 +0000 2012
>Closed-Date:    Fri Sep 30 08:22:06 +0000 2016
>Last-Modified:  Fri Sep 30 08:22:06 +0000 2016
>Originator:     Darren Reed
>Release:        NetBSD 5.99.63
>Organization:
>Environment:

>Description:
	There are two issues current with ipnat. The most serious is
that "ipnat -rf /etc/ipnat.conf" will not remove NAT rules because the
"-r" function will not work. Another side effect of this is that it is
possile to load the same rule twice - rule matching is broken. Finally,
if ipfilter is disabled and ipnat is run, it will print a very broken
error message:
:ioctl(SIOCGNATS)
whereas what it should print is this:
130003:ioctl(SIOCGNATS) ioctl on device denied, ipfitler is disabled
>How-To-Repeat:

>Fix:

Index: ip_fil_netbsd.c
===================================================================
RCS file: /room/netbsd/src/sys/dist/ipf/netinet/ip_fil_netbsd.c,v
retrieving revision 1.59
diff -c -r1.59 ip_fil_netbsd.c
*** ip_fil_netbsd.c	1 Feb 2012 02:21:19 -0000	1.59
--- ip_fil_netbsd.c	5 Feb 2012 13:01:39 -0000
***************
*** 638,644 ****
  	}

  	if (ipfmain.ipf_running <= 0) {
! 		if (unit != IPL_LOGIPF) {
  			ipfmain.ipf_interror = 130003;
  			return EIO;
  		}
--- 638,644 ----
  	}

  	if (ipfmain.ipf_running <= 0) {
! 		if (unit != IPL_LOGIPF && cmd != SIOCIPFINTERROR) {
  			ipfmain.ipf_interror = 130003;
  			return EIO;
  		}
Index: ip_nat.c
===================================================================
RCS file: /room/netbsd/src/sys/dist/ipf/netinet/ip_nat.c,v
retrieving revision 1.46
diff -c -r1.46 ip_nat.c
*** ip_nat.c	1 Feb 2012 02:21:20 -0000	1.46
--- ip_nat.c	5 Feb 2012 13:40:44 -0000
***************
*** 229,234 ****
--- 229,235 ----
  static	void	ipf_nat_addrdr(ipf_nat_softc_t *, ipnat_t *);
  static	int	ipf_nat_builddivertmp(ipf_nat_softc_t *, ipnat_t *);
  static	int	ipf_nat_clearlist(ipf_main_softc_t *, ipf_nat_softc_t *);
+ static	int	ipf_nat_cmp_rules(ipnat_t *, ipnat_t *);
  static	int	ipf_nat_decap(fr_info_t *, nat_t *);
  static	void	ipf_nat_del_active(int, u_32_t *);
  static	void	ipf_nat_del_map_mask(ipf_nat_softc_t *, int);
***************
*** 1273,1280 ****
  		MUTEX_ENTER(&softn->ipf_nat_io);
  		for (np = &softn->ipf_nat_list; ((n = *np) != NULL);
  		     np = &n->in_next)
! 			if (!bcmp((char *)&nat->in_v, (char *)&n->in_v,
! 					IPN_CMPSIZ))
  				break;
  	}

--- 1274,1280 ----
  		MUTEX_ENTER(&softn->ipf_nat_io);
  		for (np = &softn->ipf_nat_list; ((n = *np) != NULL);
  		     np = &n->in_next)
! 			if (ipf_nat_cmp_rules(nat, n) == 0)
  				break;
  	}

***************
*** 8896,8898 ****
--- 8896,8958 ----

  	RWLOCK_EXIT(&softc->ipf_nat);
  }
+ 
+ 
+ /* ------------------------------------------------------------------------ */
+ /* Function:    ipf_nat_cmp_rules                                           */
+ /* Returns:     int   - 0 == success, else rules do not match.              */
+ /* Parameters:  n1(I) - first rule to compare                               */
+ /*              n2(I) - first rule to compare                               */
+ /*                                                                          */
+ /* Compare two rules using pointers to each rule. A straight bcmp will not  */
+ /* work as some fields (such as in_dst, in_pkts) actually do change once    */
+ /* the rule has been loaded into the kernel. Whilst this function returns   */
+ /* various non-zero returns, they're strictly to aid in debugging. Use of   */
+ /* this function should simply care if the result is zero or not.           */
+ /* ------------------------------------------------------------------------ */
+ static int
+ ipf_nat_cmp_rules(ipnat_t *n1, ipnat_t *n2)
+ {
+ 	if (n1->in_size != n2->in_size)
+ 		return 1;
+ 
+ 	if (bcmp((char *)&n1->in_v, (char *)&n2->in_v,
+ 		 offsetof(ipnat_t, in_ndst) - offsetof(ipnat_t, in_v)) != 0)
+ 		return 2;
+ 
+ 	if (bcmp((char *)&n1->in_tuc, (char *)&n2->in_tuc,
+ 		 offsetof(ipnat_t, in_pkts) - offsetof(ipnat_t, in_tuc)) != 0)
+ 		return 3;
+ 	if (bcmp((char *)&n1->in_namelen, (char *)&n2->in_namelen,
+ 		 n1->in_size  - offsetof(ipnat_t, in_namelen)) != 0)
+ 		return 4;
+ 	if (n1->in_ndst.na_atype != n2->in_ndst.na_atype)
+ 		return 5;
+ 	if (n1->in_ndst.na_function != n2->in_ndst.na_function)
+ 		return 6;
+ 	if (bcmp((char *)&n1->in_ndst.na_addr, (char *)&n2->in_ndst.na_addr,
+ 		 sizeof(n1->in_ndst.na_addr)))
+ 		return 7;
+ 	if (n1->in_nsrc.na_atype != n2->in_nsrc.na_atype)
+ 		return 8;
+ 	if (n1->in_nsrc.na_function != n2->in_nsrc.na_function)
+ 		return 9;
+ 	if (bcmp((char *)&n1->in_nsrc.na_addr, (char *)&n2->in_nsrc.na_addr,
+ 		 sizeof(n1->in_nsrc.na_addr)))
+ 		return 10;
+ 	if (n1->in_odst.na_atype != n2->in_odst.na_atype)
+ 		return 11;
+ 	if (n1->in_odst.na_function != n2->in_odst.na_function)
+ 		return 12;
+ 	if (bcmp((char *)&n1->in_odst.na_addr, (char *)&n2->in_odst.na_addr,
+ 		 sizeof(n1->in_odst.na_addr)))
+ 		return 13;
+ 	if (n1->in_osrc.na_atype != n2->in_osrc.na_atype)
+ 		return 14;
+ 	if (n1->in_osrc.na_function != n2->in_osrc.na_function)
+ 		return 15;
+ 	if (bcmp((char *)&n1->in_osrc.na_addr, (char *)&n2->in_osrc.na_addr,
+ 		 sizeof(n1->in_osrc.na_addr)))
+ 		return 16;
+ 	return 0;
+ }

>Release-Note:

>Audit-Trail:
From: "Darren Reed" <darrenr@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45929 CVS commit: src/sys/dist/ipf/netinet
Date: Thu, 9 Feb 2012 07:15:28 +0000

 Module Name:	src
 Committed By:	darrenr
 Date:		Thu Feb  9 07:15:28 UTC 2012

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

 Log Message:
 PR kern/45929
 ipnat does not remove rules with -r


 To generate a diff of this commit:
 cvs rdiff -u -r1.59 -r1.60 src/sys/dist/ipf/netinet/ip_fil_netbsd.c
 cvs rdiff -u -r1.46 -r1.47 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: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/45929 CVS commit: src/sys/dist/ipf/netinet
Date: Sun, 22 Apr 2012 19:24:39 +0000

 On Thu, Feb 09, 2012 at 07:20:05AM +0000, Darren Reed wrote:
  >  Module Name:	src
  >  Committed By:	darrenr
  >  Date:		Thu Feb  9 07:15:28 UTC 2012
  >  
  >  Modified Files:
  >  	src/sys/dist/ipf/netinet: ip_fil_netbsd.c ip_nat.c
  >  
  >  Log Message:
  >  PR kern/45929
  >  ipnat does not remove rules with -r

 Can this PR be closed?

 -- 
 David A. Holland
 dholland@netbsd.org

State-Changed-From-To: open->feedback
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 07 Oct 2013 05:50:21 +0000
State-Changed-Why:
A year and a half ago, I wrote:
Can this PR be closed?


State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Fri, 30 Sep 2016 08:22:06 +0000
State-Changed-Why:
there was a commit in 2012, and no response since about whether that was a
full fix, so assume it was.


>Unformatted:

Home	PR Database Search