Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 13 Apr 2018 22:33:18 +0000 (UTC)
From:      Kristof Provost <kp@FreeBSD.org>
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
Message-ID:  <201804132233.w3DMXIEW019361@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
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;



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