From owner-dev-commits-src-main@freebsd.org Wed Jan 13 19:49:55 2021 Return-Path: Delivered-To: dev-commits-src-main@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 89FD04E6B97; Wed, 13 Jan 2021 19:49:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DGJ1v3VC9z3QHL; Wed, 13 Jan 2021 19:49:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 6AA1D1D0A0; Wed, 13 Jan 2021 19:49:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 10DJntdj089634; Wed, 13 Jan 2021 19:49:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 10DJntLw089633; Wed, 13 Jan 2021 19:49:55 GMT (envelope-from git) Date: Wed, 13 Jan 2021 19:49:55 GMT Message-Id: <202101131949.10DJntLw089633@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: ea36212bf571 - main - pf: Don't hold PF_RULES_WLOCK during copyin() on DIOCRCLRTSTATS MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: ea36212bf5711206bbaf5362a23ebb52c7f7e2a4 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-main@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for the main branch of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 13 Jan 2021 19:49:55 -0000 The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=ea36212bf5711206bbaf5362a23ebb52c7f7e2a4 commit ea36212bf5711206bbaf5362a23ebb52c7f7e2a4 Author: Kristof Provost AuthorDate: 2021-01-13 18:30:01 +0000 Commit: Kristof Provost CommitDate: 2021-01-13 18:49:42 +0000 pf: Don't hold PF_RULES_WLOCK during copyin() on DIOCRCLRTSTATS We cannot hold a non-sleepable lock during copyin(). This means we can't safely count the table, so instead we fall back to the pf_ioctl_maxcount used in other ioctls to protect against overly large requests. Reported by: syzbot+81e380344d4a6c37d78a@syzkaller.appspotmail.com MFC after: 1 week --- sys/netpfil/pf/pf_ioctl.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index b6e2ec98ddce..60c38a980d1e 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -3363,36 +3363,35 @@ DIOCCHANGEADDR_error: struct pfioc_table *io = (struct pfioc_table *)addr; struct pfr_table *pfrts; size_t totlen; - int 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); - if (n < 0) { - PF_RULES_WUNLOCK(); - error = EINVAL; + if (io->pfrio_size < 0 || io->pfrio_size > pf_ioctl_maxcount || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_table))) { + /* We used to count tables and use the minimum required + * size, so we didn't fail on overly large requests. + * Keep doing so. */ + io->pfrio_size = pf_ioctl_maxcount; break; } - 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_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();