Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 20 Jan 2021 16:24:34 GMT
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: c325366474d9 - stable/12 - pf: Don't hold PF_RULES_WLOCK during copyin() on DIOCRCLRTSTATS
Message-ID:  <202101201624.10KGOYW0049767@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=c325366474d9bb82f84c20f521b638a7166d9999

commit c325366474d9bb82f84c20f521b638a7166d9999
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2021-01-13 18:30:01 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2021-01-20 14:45:52 +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
    
    (cherry picked from commit ea36212bf5711206bbaf5362a23ebb52c7f7e2a4)
---
 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 866c4ebfaa3d..48f68fa249e2 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -3368,36 +3368,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();



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202101201624.10KGOYW0049767>