Re: IPFilter 4.1.16

[Darren Reed <darrenr@reed.wattle.id.au>]

Corey Johnston wrote:
> Update on IPF 4.1.16
>
> - It seems that ipnat -sl crashes the network.
>
> The system lists all the active ipnat rules and then tries to list the
> active sessions and then the network drops-out. All interfaces die,
> and even
> the console locks-up as it appears all network services are unavailable.
> Reboot appears to be the only remedy.
>
> - happens consistently
>
> I ran into this when trying to discover the source of the ipnat slows i
> mentioned earlier.
> I'll try and be more specific, but it looks like there is a fairly
> significant problem, at least on the Solaris build, with NAT.

The attached patch should fix this (it has for me.)

Darren
------
Merry Christmas
===============

Index: ip_nat.c
===================================================================
RCS file: /devel/CVS/IP-Filter/ip_nat.c,v
retrieving revision 2.195.2.69
diff -c -r2.195.2.69 ip_nat.c
*** ip_nat.c    18 Dec 2006 17:04:33 -0000      2.195.2.69
--- ip_nat.c    23 Dec 2006 10:57:31 -0000
***************
*** 4900,4907 ****
--- 4900,4910 ----
        ipnat_t *ipn, *nextipnat = NULL, zeroipn;
        nat_t *nat, *nextnat = NULL, zeronat;
        int error = 0, count;
+       ipftoken_t *freet;
        char *dst;
  
+       freet = NULL;
+ 
        READ_ENTER(&ipf_nat);
  
        switch (itp->igi_type)
***************
*** 4933,4938 ****
--- 4936,4942 ----
                }
                break;
        default :
+               RWLOCK_EXIT(&ipf_nat);
                return EINVAL;
        }
  
***************
*** 4942,4954 ****
                {
                case IPFGENITER_HOSTMAP :
                        if (nexthm != NULL) {
-                               /*MUTEX_ENTER(&nexthm->hm_lock);*/
-                               nexthm->hm_ref++;
-                               /*MUTEX_EXIT(&nextipnat->hm_lock);*/
                                if (nexthm->hm_next == NULL) {
!                                       ipf_freetoken(t);
                                        hm = NULL;
                                }
                        } else {
                                bzero(&zerohm, sizeof(zerohm));
                                nexthm = &zerohm;
--- 4946,4961 ----
                {
                case IPFGENITER_HOSTMAP :
                        if (nexthm != NULL) {
                                if (nexthm->hm_next == NULL) {
!                                       freet = t;
!                                       count = 1;
                                        hm = NULL;
                                }
+                               if (count == 1) {
+                                       /*MUTEX_ENTER(&nexthm->hm_lock);*/
+                                       nexthm->hm_ref++;
+                                       /*MUTEX_EXIT(&nextipnat->hm_lock);*/
+                               }
                        } else {
                                bzero(&zerohm, sizeof(zerohm));
                                nexthm = &zerohm;
***************
*** 4958,4970 ****
  
                case IPFGENITER_IPNAT :
                        if (nextipnat != NULL) {
-                               MUTEX_ENTER(&nextipnat->in_lock);
-                               nextipnat->in_use++;
-                               MUTEX_EXIT(&nextipnat->in_lock);
                                if (nextipnat->in_next == NULL) {
!                                       ipf_freetoken(t);
                                        ipn = NULL;
! 
                                }
                        } else {
                                bzero(&zeroipn, sizeof(zeroipn));
--- 4965,4979 ----
  
                case IPFGENITER_IPNAT :
                        if (nextipnat != NULL) {
                                if (nextipnat->in_next == NULL) {
!                                       freet = t;
!                                       count = 1;
                                        ipn = NULL;
!                               }
!                               if (count == 1) {
!                                       MUTEX_ENTER(&nextipnat->in_lock);
!                                       nextipnat->in_use++;
!                                       MUTEX_EXIT(&nextipnat->in_lock);
                                }
                        } else {
                                bzero(&zeroipn, sizeof(zeroipn));
***************
*** 4975,4987 ****
  
                case IPFGENITER_NAT :
                        if (nextnat != NULL) {
-                               MUTEX_ENTER(&nextnat->nat_lock);
-                               nextnat->nat_ref++;
-                               MUTEX_EXIT(&nextnat->nat_lock);
                                if (nextnat->nat_next == NULL) {
!                                       ipf_freetoken(t);
                                        nat = NULL;
                                }
                        } else {
                                bzero(&zeronat, sizeof(zeronat));
                                nextnat = &zeronat;
--- 4984,4999 ----
  
                case IPFGENITER_NAT :
                        if (nextnat != NULL) {
                                if (nextnat->nat_next == NULL) {
!                                       count = 1;
!                                       freet = t;
                                        nat = NULL;
                                }
+                               if (count == 1) {
+                                       MUTEX_ENTER(&nextnat->nat_lock);
+                                       nextnat->nat_ref++;
+                                       MUTEX_EXIT(&nextnat->nat_lock);
+                               }
                        } else {
                                bzero(&zeronat, sizeof(zeronat));
                                nextnat = &zeronat;
***************
*** 4993,4998 ****
--- 5005,5015 ----
                }
                RWLOCK_EXIT(&ipf_nat);
  
+               if (freet != NULL) {
+                       ipf_freetoken(freet);
+                       freet = NULL;
+               }
+ 
                switch (itp->igi_type)
                {
                case IPFGENITER_HOSTMAP :