Date: Sun, 5 May 1996 21:28:42 -0500 (CDT) From: Alex Nash <alex@zen.nash.org> To: FreeBSD-gnats-submit@freebsd.org Subject: kern/1175: sys/netinet/ip_fw.c race conditions Message-ID: <199605060228.VAA04410@zen.nash.org> Resent-Message-ID: <199605060241.TAA00262@freefall.freebsd.org>
index | next in thread | raw e-mail
>Number: 1175
>Category: kern
>Synopsis: faulty/lacking spl() usage in sys/netinet/ip_fw.c
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Sun May 5 19:40:01 PDT 1996
>Last-Modified:
>Originator: Alex Nash
>Organization:
>Release: FreeBSD 2.1-STABLE i386
>Environment:
options IPFIREWALL enabled
>Description:
Several locations in sys/netinet/ip_fw.c are lacking or incorrectly
use spl() functions.
The supplied diffs fix or improve:
1. IMPROVE: defer spl() until absolutely necessary.
2. FIX: attempting to delete the default (deny) policy resulted
in the priority level remaining at splnet().
3. FIX: protect LIST_REMOVE during IP_FW_FLUSH against network
interrupts. [ Rationale: Since IP_FW_FLUSH is rarely used,
the performance degredation of calling splnet() several times
to allow network interrupts through is deemed acceptable. ]
4. FIX: prevent network interrupts while clearing accounting
information. A network interrupt that occurs between clearing
the packet counter and the byte counter could result in a
zero byte count and a non-zero packet count.
>How-To-Repeat:
N/A
>Fix:
*** ip_fw.c Mon Feb 26 09:30:29 1996
--- /usr/src/sys/netinet/ip_fw.c Thu May 2 20:41:04 1996
***************
*** 444,465 ****
u_short nbr = 0;
int s;
- s = splnet();
-
fwc = malloc(sizeof *fwc, M_IPFW, M_DONTWAIT);
ftmp = malloc(sizeof *ftmp, M_IPFW, M_DONTWAIT);
if (!fwc || !ftmp) {
dprintf(("ip_fw_ctl: malloc said no\n"));
if (fwc) free(fwc, M_IPFW);
if (ftmp) free(ftmp, M_IPFW);
- splx(s);
return (ENOSPC);
}
bcopy(frwl, ftmp, sizeof(struct ip_fw));
ftmp->fw_pcnt = 0L;
ftmp->fw_bcnt = 0L;
fwc->rule = ftmp;
if (!chainptr->lh_first) {
LIST_INSERT_HEAD(chainptr, fwc, chain);
} else if (ftmp->fw_number == (u_short)-1) {
--- 444,465 ----
u_short nbr = 0;
int s;
fwc = malloc(sizeof *fwc, M_IPFW, M_DONTWAIT);
ftmp = malloc(sizeof *ftmp, M_IPFW, M_DONTWAIT);
if (!fwc || !ftmp) {
dprintf(("ip_fw_ctl: malloc said no\n"));
if (fwc) free(fwc, M_IPFW);
if (ftmp) free(ftmp, M_IPFW);
return (ENOSPC);
}
+
bcopy(frwl, ftmp, sizeof(struct ip_fw));
ftmp->fw_pcnt = 0L;
ftmp->fw_bcnt = 0L;
fwc->rule = ftmp;
+ s = splnet();
+
if (!chainptr->lh_first) {
LIST_INSERT_HEAD(chainptr, fwc, chain);
} else if (ftmp->fw_number == (u_short)-1) {
***************
*** 502,519 ****
s = splnet();
fcp = chainptr->lh_first;
! if (fcp->rule->fw_number == (u_short)-1)
! return (EINVAL);
!
! for (; fcp; fcp = fcp->chain.le_next) {
! if (fcp->rule->fw_number == frwl->fw_number) {
! LIST_REMOVE(fcp, chain);
! splx(s);
! free(fcp->rule, M_IPFW);
! free(fcp, M_IPFW);
! return 0;
}
}
splx(s);
return (EINVAL);
}
--- 502,519 ----
s = splnet();
fcp = chainptr->lh_first;
! if (fcp->rule->fw_number != (u_short)-1) {
! for (; fcp; fcp = fcp->chain.le_next) {
! if (fcp->rule->fw_number == frwl->fw_number) {
! LIST_REMOVE(fcp, chain);
! splx(s);
! free(fcp->rule, M_IPFW);
! free(fcp, M_IPFW);
! return 0;
! }
}
}
+
splx(s);
return (EINVAL);
}
***************
*** 590,596 ****
--- 590,598 ----
while (ip_fw_chain.lh_first != NULL &&
ip_fw_chain.lh_first->rule->fw_number != (u_short)-1) {
struct ip_fw_chain *fcp = ip_fw_chain.lh_first;
+ int s = splnet();
LIST_REMOVE(ip_fw_chain.lh_first, chain);
+ splx(s);
free(fcp->rule, M_IPFW);
free(fcp, M_IPFW);
}
***************
*** 598,606 ****
--- 600,610 ----
return (0);
}
if (stage == IP_FW_ZERO) {
+ int s = splnet();
struct ip_fw_chain *fcp;
for (fcp = ip_fw_chain.lh_first; fcp; fcp = fcp->chain.le_next)
fcp->rule->fw_bcnt = fcp->rule->fw_pcnt = 0;
+ splx(s);
if (m) (void)m_free(m);
return (0);
}
>Audit-Trail:
>Unformatted:
help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199605060228.VAA04410>