Author: kp
Date: Sun Jan 7 13:35:15 2018
New Revision: 327675
URL: https://svnweb.freebsd.org/changeset/base/327675
Log:
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:
head/sys/netpfil/pf/pf_ioctl.c
Modified: head/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- head/sys/netpfil/pf/pf_ioctl.c Sun Jan 7 13:21:01 2018
(r327674)
+++ head/sys/netpfil/pf/pf_ioctl.c Sun Jan 7 13:35:15 2018
(r327675)
@@ -2531,7 +2531,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);
@@ -2555,7 +2560,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);
@@ -2579,7 +2589,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);
@@ -2600,7 +2615,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);
@@ -2621,7 +2641,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);
@@ -2645,7 +2670,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);
@@ -2684,7 +2714,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);
@@ -2711,7 +2746,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);
@@ -2739,7 +2779,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);
@@ -2767,7 +2812,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);
@@ -2788,7 +2838,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);
@@ -2809,7 +2864,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);
@@ -2836,7 +2896,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);
@@ -2863,7 +2928,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);
@@ -2905,7 +2975,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);
@@ -2971,7 +3046,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);
@@ -3037,7 +3117,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);
@@ -3238,7 +3323,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"
