From owner-svn-src-stable@freebsd.org Fri Apr 13 22:33:19 2018 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 01B43F87C6D; Fri, 13 Apr 2018 22:33:19 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id AB52F7C8FB; Fri, 13 Apr 2018 22:33:18 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A65011FD65; Fri, 13 Apr 2018 22:33:18 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w3DMXIr1019364; Fri, 13 Apr 2018 22:33:18 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w3DMXIEW019361; Fri, 13 Apr 2018 22:33:18 GMT (envelope-from kp@FreeBSD.org) Message-Id: <201804132233.w3DMXIEW019361@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost Date: Fri, 13 Apr 2018 22:33:18 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r332494 - in stable/10/sys: net netpfil/pf X-SVN-Group: stable-10 X-SVN-Commit-Author: kp X-SVN-Commit-Paths: in stable/10/sys: net netpfil/pf X-SVN-Commit-Revision: 332494 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Apr 2018 22:33:19 -0000 Author: kp Date: Fri Apr 13 22:33:18 2018 New Revision: 332494 URL: https://svnweb.freebsd.org/changeset/base/332494 Log: MFC r332107: pf: Improve ioctl validation for DIOCRGETTABLES, DIOCRGETTSTATS, DIOCRCLRTSTATS and DIOCRSETTFLAGS These ioctls can process a number of items at a time, which puts us at risk of overflow in mallocarray() and of impossibly large allocations even if we don't overflow. Limit the allocation to required size (or the user allocation, if that's smaller). That does mean we need to do the allocation with the rules lock held (so the number doesn't change while we're doing this), so it can't M_WAITOK. Modified: stable/10/sys/net/pfvar.h stable/10/sys/netpfil/pf/pf_ioctl.c stable/10/sys/netpfil/pf/pf_table.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/net/pfvar.h ============================================================================== --- stable/10/sys/net/pfvar.h Fri Apr 13 22:32:28 2018 (r332493) +++ stable/10/sys/net/pfvar.h Fri Apr 13 22:33:18 2018 (r332494) @@ -1627,6 +1627,7 @@ void pfr_detach_table(struct pfr_ktable *); int pfr_clr_tables(struct pfr_table *, int *, int); int pfr_add_tables(struct pfr_table *, int, int *, int); int pfr_del_tables(struct pfr_table *, int, int *, int); +int pfr_table_count(struct pfr_table *, int); int pfr_get_tables(struct pfr_table *, struct pfr_table *, int *, int); int pfr_get_tstats(struct pfr_table *, struct pfr_tstats *, int *, int); int pfr_clr_tstats(struct pfr_table *, int, int *, int); Modified: stable/10/sys/netpfil/pf/pf_ioctl.c ============================================================================== --- stable/10/sys/netpfil/pf/pf_ioctl.c Fri Apr 13 22:32:28 2018 (r332493) +++ stable/10/sys/netpfil/pf/pf_ioctl.c Fri Apr 13 22:33:18 2018 (r332494) @@ -2571,20 +2571,25 @@ DIOCCHANGEADDR_error: case DIOCRGETTABLES: { struct pfioc_table *io = (struct pfioc_table *)addr; struct pfr_table *pfrts; - size_t totlen; + size_t totlen, n; if (io->pfrio_esize != sizeof(struct pfr_table)) { error = ENODEV; break; } + PF_RULES_RLOCK(); + n = pfr_table_count(&io->pfrio_table, io->pfrio_flags); + io->pfrio_size = min(io->pfrio_size, n); + totlen = io->pfrio_size * sizeof(struct pfr_table); + pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table), - M_TEMP, M_WAITOK); - if (! pfrts) { + M_TEMP, M_NOWAIT); + if (pfrts == NULL) { error = ENOMEM; + PF_RULES_RUNLOCK(); break; } - PF_RULES_RLOCK(); error = pfr_get_tables(&io->pfrio_table, pfrts, &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL); PF_RULES_RUNLOCK(); @@ -2597,20 +2602,24 @@ DIOCCHANGEADDR_error: case DIOCRGETTSTATS: { struct pfioc_table *io = (struct pfioc_table *)addr; struct pfr_tstats *pfrtstats; - size_t totlen; + size_t totlen, n; if (io->pfrio_esize != sizeof(struct pfr_tstats)) { error = ENODEV; break; } + PF_RULES_WLOCK(); + n = pfr_table_count(&io->pfrio_table, io->pfrio_flags); + io->pfrio_size = min(io->pfrio_size, n); + totlen = io->pfrio_size * sizeof(struct pfr_tstats); pfrtstats = mallocarray(io->pfrio_size, - sizeof(struct pfr_tstats), M_TEMP, M_WAITOK); - if (! pfrtstats) { + sizeof(struct pfr_tstats), M_TEMP, M_NOWAIT); + if (pfrtstats == NULL) { error = ENOMEM; + PF_RULES_WUNLOCK(); break; } - PF_RULES_WLOCK(); error = pfr_get_tstats(&io->pfrio_table, pfrtstats, &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL); PF_RULES_WUNLOCK(); @@ -2623,25 +2632,31 @@ DIOCCHANGEADDR_error: case DIOCRCLRTSTATS: { struct pfioc_table *io = (struct pfioc_table *)addr; struct pfr_table *pfrts; - size_t totlen; + size_t totlen, n; if (io->pfrio_esize != sizeof(struct pfr_table)) { error = ENODEV; break; } + + PF_RULES_WLOCK(); + n = pfr_table_count(&io->pfrio_table, io->pfrio_flags); + io->pfrio_size = min(io->pfrio_size, n); + totlen = io->pfrio_size * sizeof(struct pfr_table); pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table), - M_TEMP, M_WAITOK); - if (! pfrts) { + M_TEMP, M_NOWAIT); + if (pfrts == NULL) { error = ENOMEM; + PF_RULES_WUNLOCK(); break; } error = copyin(io->pfrio_buffer, pfrts, totlen); if (error) { free(pfrts, M_TEMP); + PF_RULES_WUNLOCK(); break; } - PF_RULES_WLOCK(); error = pfr_clr_tstats(pfrts, io->pfrio_size, &io->pfrio_nzero, io->pfrio_flags | PFR_FLAG_USERIOCTL); PF_RULES_WUNLOCK(); @@ -2652,25 +2667,31 @@ DIOCCHANGEADDR_error: case DIOCRSETTFLAGS: { struct pfioc_table *io = (struct pfioc_table *)addr; struct pfr_table *pfrts; - size_t totlen; + size_t totlen, n; if (io->pfrio_esize != sizeof(struct pfr_table)) { error = ENODEV; break; } + + PF_RULES_WLOCK(); + n = pfr_table_count(&io->pfrio_table, io->pfrio_flags); + io->pfrio_size = min(io->pfrio_size, n); + totlen = io->pfrio_size * sizeof(struct pfr_table); pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table), - M_TEMP, M_WAITOK); - if (! pfrts) { + M_TEMP, M_NOWAIT); + if (pfrts == NULL) { error = ENOMEM; + PF_RULES_WUNLOCK(); break; } error = copyin(io->pfrio_buffer, pfrts, totlen); if (error) { free(pfrts, M_TEMP); + PF_RULES_WUNLOCK(); break; } - PF_RULES_WLOCK(); error = pfr_set_tflags(pfrts, io->pfrio_size, io->pfrio_setflag, io->pfrio_clrflag, &io->pfrio_nchange, &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL); Modified: stable/10/sys/netpfil/pf/pf_table.c ============================================================================== --- stable/10/sys/netpfil/pf/pf_table.c Fri Apr 13 22:32:28 2018 (r332493) +++ stable/10/sys/netpfil/pf/pf_table.c Fri Apr 13 22:33:18 2018 (r332494) @@ -174,7 +174,6 @@ static struct pfr_ktable *pfr_lookup_table(struct pfr_table *); static void pfr_clean_node_mask(struct pfr_ktable *, struct pfr_kentryworkq *); -static int pfr_table_count(struct pfr_table *, int); static int pfr_skip_table(struct pfr_table *, struct pfr_ktable *, int); static struct pfr_kentry @@ -1680,7 +1679,7 @@ pfr_fix_anchor(char *anchor) return (0); } -static int +int pfr_table_count(struct pfr_table *filter, int flags) { struct pf_ruleset *rs;