git: 2b82c57e3997 - main - pf: Initialize arrays before copying out to userland

Mark Johnston markj at FreeBSD.org
Wed Jul 28 14:41:32 UTC 2021


The branch main has been updated by markj:

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

commit 2b82c57e399700f5134c47d86d45ef2f2bd465e2
Author:     Mark Johnston <markj at FreeBSD.org>
AuthorDate: 2021-07-28 14:16:25 +0000
Commit:     Mark Johnston <markj at FreeBSD.org>
CommitDate: 2021-07-28 14:40:49 +0000

    pf: Initialize arrays before copying out to userland
    
    A number of pf ioctls populate an array of structures and copy it out.
    They have the following structures:
    - caller specifies the size of its output buffer
    - ioctl handler allocates a kernel buffer of the same size
    - ioctl handler populates the buffer, possibly leaving some items
      initialized if the caller provided more space than needed
    - ioctl handler copies the entire buffer out to userland
    
    Thus, if more space was provided than is required, we end up copying out
    uninitialized kernel memory.  Simply zero the buffer at allocation time
    to prevent this.
    
    Reported by:    KMSAN
    Reviewed by:    kp
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D31313
---
 sys/netpfil/pf/pf_ioctl.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c
index 33c38eaaf9fa..3dc52da05606 100644
--- a/sys/netpfil/pf/pf_ioctl.c
+++ b/sys/netpfil/pf/pf_ioctl.c
@@ -3824,7 +3824,7 @@ DIOCCHANGEADDR_error:
 		totlen = io->pfrio_size * sizeof(struct pfr_table);
 
 		pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
-		    M_TEMP, M_NOWAIT);
+		    M_TEMP, M_NOWAIT | M_ZERO);
 		if (pfrts == NULL) {
 			error = ENOMEM;
 			PF_RULES_RUNLOCK();
@@ -3862,7 +3862,7 @@ DIOCCHANGEADDR_error:
 
 		totlen = io->pfrio_size * sizeof(struct pfr_tstats);
 		pfrtstats = mallocarray(io->pfrio_size,
-		    sizeof(struct pfr_tstats), M_TEMP, M_NOWAIT);
+		    sizeof(struct pfr_tstats), M_TEMP, M_NOWAIT | M_ZERO);
 		if (pfrtstats == NULL) {
 			error = ENOMEM;
 			PF_RULES_RUNLOCK();
@@ -4094,7 +4094,7 @@ DIOCCHANGEADDR_error:
 		}
 		totlen = io->pfrio_size * sizeof(struct pfr_addr);
 		pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
-		    M_TEMP, M_WAITOK);
+		    M_TEMP, M_WAITOK | M_ZERO);
 		PF_RULES_RLOCK();
 		error = pfr_get_addrs(&io->pfrio_table, pfras,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -4122,7 +4122,7 @@ DIOCCHANGEADDR_error:
 		}
 		totlen = io->pfrio_size * sizeof(struct pfr_astats);
 		pfrastats = mallocarray(io->pfrio_size,
-		    sizeof(struct pfr_astats), M_TEMP, M_WAITOK);
+		    sizeof(struct pfr_astats), M_TEMP, M_WAITOK | M_ZERO);
 		PF_RULES_RLOCK();
 		error = pfr_get_astats(&io->pfrio_table, pfrastats,
 		    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -4626,7 +4626,7 @@ DIOCCHANGEADDR_error:
 
 		bufsiz = io->pfiio_size * sizeof(struct pfi_kif);
 		ifstore = mallocarray(io->pfiio_size, sizeof(struct pfi_kif),
-		    M_TEMP, M_WAITOK);
+		    M_TEMP, M_WAITOK | M_ZERO);
 
 		PF_RULES_RLOCK();
 		pfi_get_ifaces(io->pfiio_name, ifstore, &io->pfiio_size);


More information about the dev-commits-src-all mailing list