Author: kp
Date: Tue Jan 23 04:30:46 2018
New Revision: 328275
URL: https://svnweb.freebsd.org/changeset/base/328275

Log:
  MFC r327675
  
  pf: Avoid integer overflow issues by using mallocarray() iso. malloc()
  
  pfioctl() handles several ioctl that takes variable length input, these
  include:
  - DIOCRADDTABLES
  - DIOCRDELTABLES
  - DIOCRGETTABLES
  - DIOCRGETTSTATS
  - DIOCRCLRTSTATS
  - DIOCRSETTFLAGS
  
  All of them take a pfioc_table struct as input from userland. One of
  its elements (pfrio_size) is used in a buffer length calculation.
  The calculation contains an integer overflow which if triggered can lead
  to out of bound reads and writes later on.
  
  Reported by:  Ilja Van Sprundel <ivansprun...@ioactive.com>

Modified:
  stable/11/sys/netpfil/pf/pf_ioctl.c
Directory Properties:
  stable/11/   (props changed)

Modified: stable/11/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- stable/11/sys/netpfil/pf/pf_ioctl.c Tue Jan 23 04:01:48 2018        
(r328274)
+++ stable/11/sys/netpfil/pf/pf_ioctl.c Tue Jan 23 04:30:46 2018        
(r328275)
@@ -2524,7 +2524,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_table);
-               pfrts = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+                   M_TEMP, M_WAITOK);
+               if (! pfrts) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfrts, totlen);
                if (error) {
                        free(pfrts, M_TEMP);
@@ -2548,7 +2553,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_table);
-               pfrts = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+                   M_TEMP, M_WAITOK);
+               if (! pfrts) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfrts, totlen);
                if (error) {
                        free(pfrts, M_TEMP);
@@ -2572,7 +2582,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_table);
-               pfrts = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+                   M_TEMP, M_WAITOK);
+               if (! pfrts) {
+                       error = ENOMEM;
+                       break;
+               }
                PF_RULES_RLOCK();
                error = pfr_get_tables(&io->pfrio_table, pfrts,
                    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -2593,7 +2608,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_tstats);
-               pfrtstats = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrtstats = mallocarray(io->pfrio_size,
+                   sizeof(struct pfr_tstats), M_TEMP, M_WAITOK);
+               if (! pfrtstats) {
+                       error = ENOMEM;
+                       break;
+               }
                PF_RULES_WLOCK();
                error = pfr_get_tstats(&io->pfrio_table, pfrtstats,
                    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -2614,7 +2634,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_table);
-               pfrts = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+                   M_TEMP, M_WAITOK);
+               if (! pfrts) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfrts, totlen);
                if (error) {
                        free(pfrts, M_TEMP);
@@ -2638,7 +2663,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_table);
-               pfrts = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
+                   M_TEMP, M_WAITOK);
+               if (! pfrts) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfrts, totlen);
                if (error) {
                        free(pfrts, M_TEMP);
@@ -2677,7 +2707,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2704,7 +2739,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2732,7 +2772,12 @@ DIOCCHANGEADDR_error:
                }
                count = max(io->pfrio_size, io->pfrio_size2);
                totlen = count * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(count, sizeof(struct pfr_addr), M_TEMP,
+                   M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2760,7 +2805,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                PF_RULES_RLOCK();
                error = pfr_get_addrs(&io->pfrio_table, pfras,
                    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -2781,7 +2831,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_astats);
-               pfrastats = malloc(totlen, M_TEMP, M_WAITOK);
+               pfrastats = mallocarray(io->pfrio_size,
+                   sizeof(struct pfr_astats), M_TEMP, M_WAITOK);
+               if (! pfrastats) {
+                       error = ENOMEM;
+                       break;
+               }
                PF_RULES_RLOCK();
                error = pfr_get_astats(&io->pfrio_table, pfrastats,
                    &io->pfrio_size, io->pfrio_flags | PFR_FLAG_USERIOCTL);
@@ -2802,7 +2857,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2829,7 +2889,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2856,7 +2921,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = io->pfrio_size * sizeof(struct pfr_addr);
-               pfras = malloc(totlen, M_TEMP, M_WAITOK);
+               pfras = mallocarray(io->pfrio_size, sizeof(struct pfr_addr),
+                   M_TEMP, M_WAITOK);
+               if (! pfras) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->pfrio_buffer, pfras, totlen);
                if (error) {
                        free(pfras, M_TEMP);
@@ -2898,7 +2968,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = sizeof(struct pfioc_trans_e) * io->size;
-               ioes = malloc(totlen, M_TEMP, M_WAITOK);
+               ioes = mallocarray(io->size, sizeof(struct pfioc_trans_e),
+                   M_TEMP, M_WAITOK);
+               if (! ioes) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->array, ioes, totlen);
                if (error) {
                        free(ioes, M_TEMP);
@@ -2964,7 +3039,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = sizeof(struct pfioc_trans_e) * io->size;
-               ioes = malloc(totlen, M_TEMP, M_WAITOK);
+               ioes = mallocarray(io->size, sizeof(struct pfioc_trans_e),
+                   M_TEMP, M_WAITOK);
+               if (! ioes) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->array, ioes, totlen);
                if (error) {
                        free(ioes, M_TEMP);
@@ -3030,7 +3110,12 @@ DIOCCHANGEADDR_error:
                        break;
                }
                totlen = sizeof(struct pfioc_trans_e) * io->size;
-               ioes = malloc(totlen, M_TEMP, M_WAITOK);
+               ioes = mallocarray(io->size, sizeof(struct pfioc_trans_e),
+                   M_TEMP, M_WAITOK);
+               if (! ioes) {
+                       error = ENOMEM;
+                       break;
+               }
                error = copyin(io->array, ioes, totlen);
                if (error) {
                        free(ioes, M_TEMP);
@@ -3231,7 +3316,12 @@ DIOCCHANGEADDR_error:
                }
 
                bufsiz = io->pfiio_size * sizeof(struct pfi_kif);
-               ifstore = malloc(bufsiz, M_TEMP, M_WAITOK);
+               ifstore = mallocarray(io->pfiio_size, sizeof(struct pfi_kif),
+                   M_TEMP, M_WAITOK);
+               if (! ifstore) {
+                       error = ENOMEM;
+                       break;
+               }
                PF_RULES_RLOCK();
                pfi_get_ifaces(io->pfiio_name, ifstore, &io->pfiio_size);
                PF_RULES_RUNLOCK();
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to