Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 6 Apr 2018 19:20:45 +0000 (UTC)
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r332136 - head/sys/netpfil/pf
Message-ID:  <201804061920.w36JKj4b022968@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: kp
Date: Fri Apr  6 19:20:45 2018
New Revision: 332136
URL: https://svnweb.freebsd.org/changeset/base/332136

Log:
  pf: Improve ioctl validation for DIOCIGETIFACES and DIOCXCOMMIT
  
  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.
  
  There's no obvious limit to the request size for these, so we limit the
  requests to something which won't overflow. Change the memory allocation
  to M_NOWAIT so excessive requests will fail rather than stall forever.
  
  MFC after:	1 week

Modified:
  head/sys/netpfil/pf/pf_ioctl.c

Modified: head/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- head/sys/netpfil/pf/pf_ioctl.c	Fri Apr  6 19:17:59 2018	(r332135)
+++ head/sys/netpfil/pf/pf_ioctl.c	Fri Apr  6 19:20:45 2018	(r332136)
@@ -3143,10 +3143,17 @@ DIOCCHANGEADDR_error:
 			error = ENODEV;
 			break;
 		}
+
+		if (io->size < 0 ||
+		    WOULD_OVERFLOW(io->size, sizeof(struct pfioc_trans_e))) {
+			error = EINVAL;
+			break;
+		}
+
 		totlen = sizeof(struct pfioc_trans_e) * io->size;
 		ioes = mallocarray(io->size, sizeof(struct pfioc_trans_e),
-		    M_TEMP, M_WAITOK);
-		if (! ioes) {
+		    M_TEMP, M_NOWAIT);
+		if (ioes == NULL) {
 			error = ENOMEM;
 			break;
 		}
@@ -3349,13 +3356,20 @@ DIOCCHANGEADDR_error:
 			break;
 		}
 
+		if (io->pfiio_size < 0 ||
+		    WOULD_OVERFLOW(io->pfiio_size, sizeof(struct pfi_kif))) {
+			error = EINVAL;
+			break;
+		}
+
 		bufsiz = io->pfiio_size * sizeof(struct pfi_kif);
 		ifstore = mallocarray(io->pfiio_size, sizeof(struct pfi_kif),
-		    M_TEMP, M_WAITOK);
-		if (! ifstore) {
+		    M_TEMP, M_NOWAIT);
+		if (ifstore == NULL) {
 			error = ENOMEM;
 			break;
 		}
+
 		PF_RULES_RLOCK();
 		pfi_get_ifaces(io->pfiio_name, ifstore, &io->pfiio_size);
 		PF_RULES_RUNLOCK();

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