From owner-freebsd-bugs Sun May 5 19:42:45 1996 Return-Path: owner-bugs Received: (from root@localhost) by freefall.freebsd.org (8.7.3/8.7.3) id TAA00326 for bugs-outgoing; Sun, 5 May 1996 19:42:45 -0700 (PDT) Received: (from gnats@localhost) by freefall.freebsd.org (8.7.3/8.7.3) id TAA00262 Sun, 5 May 1996 19:41:24 -0700 (PDT) Resent-Date: Sun, 5 May 1996 19:41:24 -0700 (PDT) Resent-Message-Id: <199605060241.TAA00262@freefall.freebsd.org> Resent-From: gnats (GNATS Management) Resent-To: freebsd-bugs Resent-Reply-To: FreeBSD-gnats@freefall.FreeBSD.org, alex@zen.nash.org Received: from zen.nash.org (nash.pr.mcs.net [204.95.47.72]) by freefall.freebsd.org (8.7.3/8.7.3) with ESMTP id TAA29363 for ; Sun, 5 May 1996 19:29:05 -0700 (PDT) Received: (from alex@localhost) by zen.nash.org (8.7.5/8.6.12) id VAA04410; Sun, 5 May 1996 21:28:42 -0500 (CDT) Message-Id: <199605060228.VAA04410@zen.nash.org> Date: Sun, 5 May 1996 21:28:42 -0500 (CDT) From: Alex Nash Reply-To: alex@zen.nash.org To: FreeBSD-gnats-submit@freebsd.org X-Send-Pr-Version: 3.2 Subject: kern/1175: sys/netinet/ip_fw.c race conditions Sender: owner-bugs@freebsd.org X-Loop: FreeBSD.org Precedence: bulk >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: