From 1a125a2f7f4f322fbf263bba5498d7a261ae93b4 Mon Sep 17 00:00:00 2001 From: Kristof Provost Date: Fri, 6 Apr 2018 19:36:35 +0000 Subject: [PATCH] pf: Improve ioctl validation Ensure that multiplications for memory allocations cannot overflow, and that we'll not try to allocate M_WAITOK for potentially overly large allocations. MFC after: 1 week --- sys/netpfil/pf/pf_ioctl.c | 73 +++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 10 deletions(-) diff --git a/sys/netpfil/pf/pf_ioctl.c b/sys/netpfil/pf/pf_ioctl.c index 4d8f576bfa4..2626fc6b55b 100644 --- a/sys/netpfil/pf/pf_ioctl.c +++ b/sys/netpfil/pf/pf_ioctl.c @@ -2740,9 +2740,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2772,9 +2777,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2804,10 +2814,18 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || io->pfrio_size2 < 0) { + error = EINVAL; + break; + } count = max(io->pfrio_size, io->pfrio_size2); + if (WOULD_OVERFLOW(count, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = count * sizeof(struct pfr_addr); pfras = mallocarray(count, sizeof(struct pfr_addr), M_TEMP, - M_WAITOK); + M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2838,9 +2856,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2864,9 +2887,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_astats))) { + error = EINVAL; + break; + } 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_NOWAIT); if (! pfrastats) { error = ENOMEM; break; @@ -2890,9 +2918,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2922,9 +2955,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -2954,9 +2992,14 @@ DIOCCHANGEADDR_error: error = ENODEV; break; } + if (io->pfrio_size < 0 || + WOULD_OVERFLOW(io->pfrio_size, sizeof(struct pfr_addr))) { + error = EINVAL; + break; + } totlen = io->pfrio_size * sizeof(struct pfr_addr); pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr), - M_TEMP, M_WAITOK); + M_TEMP, M_NOWAIT); if (! pfras) { error = ENOMEM; break; @@ -3001,9 +3044,14 @@ 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); + M_TEMP, M_NOWAIT); if (! ioes) { error = ENOMEM; break; @@ -3072,9 +3120,14 @@ 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); + M_TEMP, M_NOWAIT); if (! ioes) { error = ENOMEM; break;