From e94bd5ada25b0e08188acabe9df8e8733cfea926 Mon Sep 17 00:00:00 2001 From: darrenr Date: Tue, 30 Oct 2007 15:23:27 +0000 Subject: [PATCH] Apply a few changes from ipfilter-current: * Do not hold any locks over calls to copyin/copyout. * Clean up some #ifdefs * fix a possible mbuf leak when NAT fails on policy routed packets PR: 117216 --- sys/contrib/ipfilter/netinet/fil.c | 8 +-- sys/contrib/ipfilter/netinet/ip_auth.c | 2 +- sys/contrib/ipfilter/netinet/ip_compat.h | 2 +- sys/contrib/ipfilter/netinet/ip_fil_freebsd.c | 13 ++-- sys/contrib/ipfilter/netinet/ip_log.c | 11 +-- sys/contrib/ipfilter/netinet/ip_nat.c | 70 ++++++++++++------- sys/contrib/ipfilter/netinet/ip_state.c | 7 +- 7 files changed, 69 insertions(+), 44 deletions(-) diff --git a/sys/contrib/ipfilter/netinet/fil.c b/sys/contrib/ipfilter/netinet/fil.c index 5a083a445a2d..9c6d6ea9e854 100644 --- a/sys/contrib/ipfilter/netinet/fil.c +++ b/sys/contrib/ipfilter/netinet/fil.c @@ -2514,7 +2514,7 @@ int out; } else #endif { -#if (defined(OpenBSD) && OpenBSD >= 200311) && defined(_KERNEL) +#if (defined(OpenBSD) && (OpenBSD >= 200311)) && defined(_KERNEL) ip->ip_len = ntohs(ip->ip_len); ip->ip_off = ntohs(ip->ip_off); #endif @@ -2777,7 +2777,7 @@ finished: RWLOCK_EXIT(&ipf_global); #ifdef _KERNEL -# if defined(OpenBSD) && OpenBSD >= 200311 +# if (defined(OpenBSD) && (OpenBSD >= 200311)) if (FR_ISPASS(pass) && (v == 4)) { ip = fin->fin_ip; ip->ip_len = ntohs(ip->ip_len); @@ -7024,7 +7024,6 @@ void *ctx; break; } - RWLOCK_EXIT(&ipf_global); WRITE_ENTER(&ipf_global); if (tmp) { if (fr_running > 0) @@ -7040,6 +7039,7 @@ void *ctx; if (error == 0) fr_running = -1; } + RWLOCK_EXIT(&ipf_global); } break; @@ -7181,7 +7181,6 @@ void *ctx; if (!(mode & FWRITE)) error = EPERM; else { - RWLOCK_EXIT(&ipf_global); WRITE_ENTER(&ipf_global); #ifdef MENTAT error = ipfsync(); @@ -7189,6 +7188,7 @@ void *ctx; frsync(NULL); error = 0; #endif + RWLOCK_EXIT(&ipf_global); } break; diff --git a/sys/contrib/ipfilter/netinet/ip_auth.c b/sys/contrib/ipfilter/netinet/ip_auth.c index 6884110eb170..fddb0438eed6 100644 --- a/sys/contrib/ipfilter/netinet/ip_auth.c +++ b/sys/contrib/ipfilter/netinet/ip_auth.c @@ -53,7 +53,7 @@ struct file; # include #endif #if (defined(_BSDI_VERSION) && _BSDI_VERSION >= 199802) || \ - (__FreeBSD_version >= 400000) + (defined(__FreeBSD_version) &&(__FreeBSD_version >= 400000)) # include #endif #if defined(__NetBSD__) || defined(__OpenBSD__) || defined(bsdi) diff --git a/sys/contrib/ipfilter/netinet/ip_compat.h b/sys/contrib/ipfilter/netinet/ip_compat.h index b65300754761..97c9d7e22016 100644 --- a/sys/contrib/ipfilter/netinet/ip_compat.h +++ b/sys/contrib/ipfilter/netinet/ip_compat.h @@ -35,7 +35,7 @@ #ifndef SOLARIS #define SOLARIS (defined(sun) && (defined(__svr4__) || defined(__SVR4))) #endif -#if defined(SOLARIS2) && SOLARIS2 >= 8 +#if (defined(SOLARIS2) && (SOLARIS2 >= 8)) # ifndef USE_INET6 # define USE_INET6 # endif diff --git a/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c b/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c index 84281ab16260..0f39afa268eb 100644 --- a/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c +++ b/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c @@ -341,22 +341,19 @@ int mode; if (unit != IPL_LOGIPF) return EIO; if (cmd != SIOCIPFGETNEXT && cmd != SIOCIPFGET && - cmd != SIOCIPFSET && cmd != SIOCFRENB && + cmd != SIOCIPFSET && cmd != SIOCFRENB && cmd != SIOCGETFS && cmd != SIOCGETFF) return EIO; } SPL_NET(s); - READ_ENTER(&ipf_global); error = fr_ioctlswitch(unit, data, cmd, mode, p->p_uid, p); if (error != -1) { - RWLOCK_EXIT(&ipf_global); SPL_X(s); return error; } - RWLOCK_EXIT(&ipf_global); SPL_X(s); return error; @@ -885,13 +882,17 @@ iplinit() #endif /* __FreeBSD_version < 300000 */ +/* + * m0 - pointer to mbuf where the IP packet starts + * mpp - pointer to the mbuf pointer that is the start of the mbuf chain + */ int fr_fastroute(m0, mpp, fin, fdp) mb_t *m0, **mpp; fr_info_t *fin; frdest_t *fdp; { register struct ip *ip, *mhip; - register struct mbuf *m = m0; + register struct mbuf *m = *mpp; register struct route *ro; int len, off, error = 0, hlen, code; struct ifnet *ifp, *sifp; @@ -1016,7 +1017,7 @@ frdest_t *fdp; break; case -1 : error = -1; - goto done; + goto bad; break; } diff --git a/sys/contrib/ipfilter/netinet/ip_log.c b/sys/contrib/ipfilter/netinet/ip_log.c index 583eb6399a0f..863cc9c058e2 100644 --- a/sys/contrib/ipfilter/netinet/ip_log.c +++ b/sys/contrib/ipfilter/netinet/ip_log.c @@ -52,7 +52,8 @@ struct file; # undef _KERNEL # undef KERNEL #endif -#if __FreeBSD_version >= 220000 && defined(_KERNEL) +#if (defined(__FreeBSD_version) && (__FreeBSD_version >= 220000)) && \ + defined(_KERNEL) # include # include #else @@ -61,14 +62,14 @@ struct file; #include #if defined(_KERNEL) # include -# if defined(NetBSD) && (__NetBSD_Version__ >= 104000000) +# if (defined(NetBSD) && (__NetBSD_Version__ >= 104000000)) # include # endif #endif /* _KERNEL */ #if !SOLARIS && !defined(__hpux) && !defined(linux) -# if (defined(NetBSD) && NetBSD > 199609) || \ - (defined(OpenBSD) && OpenBSD > 199603) || \ - (__FreeBSD_version >= 300000) +# if (defined(NetBSD) && (NetBSD > 199609)) || \ + (defined(OpenBSD) && (OpenBSD > 199603)) || \ + (defined(__FreeBSD_version) && (__FreeBSD_version >= 300000)) # include # else # include diff --git a/sys/contrib/ipfilter/netinet/ip_nat.c b/sys/contrib/ipfilter/netinet/ip_nat.c index 23cb57989b5b..a6963213a0a5 100644 --- a/sys/contrib/ipfilter/netinet/ip_nat.c +++ b/sys/contrib/ipfilter/netinet/ip_nat.c @@ -190,8 +190,8 @@ static void nat_addnat __P((struct ipnat *)); static void nat_addrdr __P((struct ipnat *)); static void nat_delrdr __P((struct ipnat *)); static void nat_delnat __P((struct ipnat *)); -static int fr_natgetent __P((caddr_t)); -static int fr_natgetsz __P((caddr_t)); +static int fr_natgetent __P((caddr_t, int)); +static int fr_natgetsz __P((caddr_t, int)); static int fr_natputent __P((caddr_t, int)); static int nat_extraflush __P((int)); static int nat_gettable __P((char *)); @@ -820,20 +820,23 @@ void *ctx; { natlookup_t nl; - if (getlock) { - READ_ENTER(&ipf_nat); - } error = fr_inobj(data, &nl, IPFOBJ_NATLOOKUP); if (error == 0) { - if (nat_lookupredir(&nl) != NULL) { + void *ptr; + + if (getlock) { + READ_ENTER(&ipf_nat); + } + ptr = nat_lookupredir(&nl); + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + } + if (ptr != NULL) { error = fr_outobj(data, &nl, IPFOBJ_NATLOOKUP); } else { error = ESRCH; } } - if (getlock) { - RWLOCK_EXIT(&ipf_nat); - } break; } @@ -888,26 +891,14 @@ void *ctx; case SIOCSTGSZ : if (fr_nat_lock) { - if (getlock) { - READ_ENTER(&ipf_nat); - } - error = fr_natgetsz(data); - if (getlock) { - RWLOCK_EXIT(&ipf_nat); - } + error = fr_natgetsz(data, getlock); } else error = EACCES; break; case SIOCSTGET : if (fr_nat_lock) { - if (getlock) { - READ_ENTER(&ipf_nat); - } - error = fr_natgetent(data); - if (getlock) { - RWLOCK_EXIT(&ipf_nat); - } + error = fr_natgetent(data, getlock); } else error = EACCES; break; @@ -1202,8 +1193,9 @@ int getlock; /* The size of the entry is stored in the ng_sz field and the enture natget */ /* structure is copied back to the user. */ /* ------------------------------------------------------------------------ */ -static int fr_natgetsz(data) +static int fr_natgetsz(data, getlock) caddr_t data; +int getlock; { ap_session_t *aps; nat_t *nat, *n; @@ -1212,6 +1204,10 @@ caddr_t data; if (BCOPYIN(data, &ng, sizeof(ng)) != 0) return EFAULT; + if (getlock) { + READ_ENTER(&ipf_nat); + } + nat = ng.ng_ptr; if (!nat) { nat = nat_instances; @@ -1220,6 +1216,9 @@ caddr_t data; * Empty list so the size returned is 0. Simple. */ if (nat == NULL) { + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + } if (BCOPYOUT(&ng, data, sizeof(ng)) != 0) return EFAULT; return 0; @@ -1233,8 +1232,12 @@ caddr_t data; for (n = nat_instances; n; n = n->nat_next) if (n == nat) break; - if (!n) + if (n == NULL) { + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + } return ESRCH; + } } /* @@ -1247,6 +1250,9 @@ caddr_t data; if (aps->aps_data != 0) ng.ng_sz += aps->aps_psiz; } + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + } if (BCOPYOUT(&ng, data, sizeof(ng)) != 0) return EFAULT; @@ -1264,8 +1270,9 @@ caddr_t data; /* Copies out NAT entry to user space. Any additional data held for a */ /* proxy is also copied, as to is the NAT rule which was responsible for it */ /* ------------------------------------------------------------------------ */ -static int fr_natgetent(data) +static int fr_natgetent(data, getlock) caddr_t data; +int getlock; { int error, outsize; ap_session_t *aps; @@ -1283,6 +1290,10 @@ caddr_t data; if (ipn == NULL) return ENOMEM; + if (getlock) { + READ_ENTER(&ipf_nat); + } + ipn->ipn_dsize = ipns.ipn_dsize; nat = ipns.ipn_next; if (nat == NULL) { @@ -1353,10 +1364,17 @@ caddr_t data; error = ENOBUFS; } if (error == 0) { + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + getlock = 0; + } error = fr_outobjsz(data, ipn, IPFOBJ_NATSAVE, ipns.ipn_dsize); } finished: + if (getlock) { + RWLOCK_EXIT(&ipf_nat); + } if (ipn != NULL) { KFREES(ipn, ipns.ipn_dsize); } diff --git a/sys/contrib/ipfilter/netinet/ip_state.c b/sys/contrib/ipfilter/netinet/ip_state.c index cfb79749ef95..aa9192afee13 100644 --- a/sys/contrib/ipfilter/netinet/ip_state.c +++ b/sys/contrib/ipfilter/netinet/ip_state.c @@ -655,10 +655,12 @@ caddr_t data; if (error != 0) return error; + READ_ENTER(&ipf_state); isn = ips.ips_next; if (isn == NULL) { isn = ips_list; if (isn == NULL) { + RWLOCK_EXIT(&ipf_state); if (ips.ips_next == NULL) return ENOENT; return 0; @@ -672,8 +674,10 @@ caddr_t data; for (is = ips_list; is; is = is->is_next) if (is == isn) break; - if (!is) + if (is == NULL) { + RWLOCK_EXIT(&ipf_state); return ESRCH; + } } ips.ips_next = isn->is_next; bcopy((char *)isn, (char *)&ips.ips_is, sizeof(ips.ips_is)); @@ -681,6 +685,7 @@ caddr_t data; if (isn->is_rule != NULL) bcopy((char *)isn->is_rule, (char *)&ips.ips_fr, sizeof(ips.ips_fr)); + RWLOCK_EXIT(&ipf_state); error = fr_outobj(data, &ips, IPFOBJ_STATESAVE); return error; }