From fb0e1892d9e31cdba013c27b24c47c86838924a8 Mon Sep 17 00:00:00 2001 From: Enji Cooper Date: Fri, 22 Apr 2016 18:05:34 +0000 Subject: [PATCH] Fix up pointer issues with lib/libkvm In particular, - avoid dereferencing NULL pointers - test pointers against NULL, not 0 - test for errout == NULL in the top-level functions (kvm_open, kvm_openfiles, kvm_open2, etc) - Replace a realloc and free on failure with reallocf Found with: devel/cocchinelle Differential Revision: https://reviews.freebsd.org/D5954 MFC after: 1 week Reviewed by: jhb Sponsored by: EMC / Isilon Storage Division --- lib/libkvm/kvm.c | 22 +++++++++++++--------- lib/libkvm/kvm_amd64.c | 8 ++++++-- lib/libkvm/kvm_arm.c | 6 +++++- lib/libkvm/kvm_i386.c | 12 ++++++++++-- lib/libkvm/kvm_minidump_aarch64.c | 2 +- lib/libkvm/kvm_minidump_amd64.c | 2 +- lib/libkvm/kvm_minidump_arm.c | 2 +- lib/libkvm/kvm_minidump_i386.c | 2 +- lib/libkvm/kvm_minidump_mips.c | 2 +- lib/libkvm/kvm_proc.c | 29 +++++++++++++---------------- 10 files changed, 52 insertions(+), 35 deletions(-) diff --git a/lib/libkvm/kvm.c b/lib/libkvm/kvm.c index 4cbda47433e3..de21f39d1eab 100644 --- a/lib/libkvm/kvm.c +++ b/lib/libkvm/kvm.c @@ -379,12 +379,12 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, int flag, char *errout) kd->vmfd = -1; kd->pmfd = -1; kd->nlfd = -1; - kd->vmst = 0; - kd->procbase = 0; - kd->argspc = 0; - kd->argv = 0; + kd->vmst = NULL; + kd->procbase = NULL; + kd->argspc = NULL; + kd->argv = NULL; - if (uf == 0) + if (uf == NULL) uf = getbootfile(); else if (strlen(uf) >= MAXPATHLEN) { _kvm_err(kd, kd->program, "exec file name too long"); @@ -394,7 +394,7 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, int flag, char *errout) _kvm_err(kd, kd->program, "bad flags arg"); goto failed; } - if (mf == 0) + if (mf == NULL) mf = _PATH_MEM; if ((kd->pmfd = open(mf, flag | O_CLOEXEC, 0)) < 0) { @@ -471,7 +471,7 @@ _kvm_open(kvm_t *kd, const char *uf, const char *mf, int flag, char *errout) /* * Copy out the error if doing sane error semantics. */ - if (errout != 0) + if (errout != NULL) strlcpy(errout, kd->errbuf, _POSIX2_LINE_MAX); (void)kvm_close(kd); return (0); @@ -484,7 +484,9 @@ kvm_openfiles(const char *uf, const char *mf, const char *sf __unused, int flag, kvm_t *kd; if ((kd = calloc(1, sizeof(*kd))) == NULL) { - (void)strlcpy(errout, strerror(errno), _POSIX2_LINE_MAX); + if (errout != NULL) + (void)strlcpy(errout, strerror(errno), + _POSIX2_LINE_MAX); return (0); } return (_kvm_open(kd, uf, mf, flag, errout)); @@ -513,7 +515,9 @@ kvm_open2(const char *uf, const char *mf, int flag, char *errout, kvm_t *kd; if ((kd = calloc(1, sizeof(*kd))) == NULL) { - (void)strlcpy(errout, strerror(errno), _POSIX2_LINE_MAX); + if (errout != NULL) + (void)strlcpy(errout, strerror(errno), + _POSIX2_LINE_MAX); return (0); } kd->resolve_symbol = resolver; diff --git a/lib/libkvm/kvm_amd64.c b/lib/libkvm/kvm_amd64.c index 70789f4c989d..57a8728645a3 100644 --- a/lib/libkvm/kvm_amd64.c +++ b/lib/libkvm/kvm_amd64.c @@ -118,7 +118,7 @@ _amd64_initvtop(kvm_t *kd) amd64_pml4e_t *PML4; kd->vmst = (struct vmstate *)_kvm_malloc(kd, sizeof(*kd->vmst)); - if (kd->vmst == 0) { + if (kd->vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } @@ -153,6 +153,10 @@ _amd64_initvtop(kvm_t *kd) } pa = le64toh(pa); PML4 = _kvm_malloc(kd, AMD64_PAGE_SIZE); + if (PML4 == NULL) { + _kvm_err(kd, kd->program, "cannot allocate PML4"); + return (-1); + } if (kvm_read2(kd, pa, PML4, AMD64_PAGE_SIZE) != AMD64_PAGE_SIZE) { _kvm_err(kd, kd->program, "cannot read KPML4phys"); return (-1); @@ -188,7 +192,7 @@ _amd64_vatop(kvm_t *kd, kvaddr_t va, off_t *pa) * If we are initializing (kernel page table descriptor pointer * not yet set) then return pa == va to avoid infinite recursion. */ - if (vm->PML4 == 0) { + if (vm->PML4 == NULL) { s = _kvm_pa2off(kd, va, pa); if (s == 0) { _kvm_err(kd, kd->program, diff --git a/lib/libkvm/kvm_arm.c b/lib/libkvm/kvm_arm.c index ae5fb5bc8646..eeda900063f2 100644 --- a/lib/libkvm/kvm_arm.c +++ b/lib/libkvm/kvm_arm.c @@ -117,7 +117,7 @@ _arm_initvtop(kvm_t *kd) } vm = _kvm_malloc(kd, sizeof(*vm)); - if (vm == 0) { + if (vm == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } @@ -168,6 +168,10 @@ _arm_initvtop(kvm_t *kd) return (-1); } l1pt = _kvm_malloc(kd, ARM_L1_TABLE_SIZE); + if (l1pt == NULL) { + _kvm_err(kd, kd->program, "cannot allocate l1pt"); + return (-1); + } if (kvm_read2(kd, pa, l1pt, ARM_L1_TABLE_SIZE) != ARM_L1_TABLE_SIZE) { _kvm_err(kd, kd->program, "cannot read l1pt"); free(l1pt); diff --git a/lib/libkvm/kvm_i386.c b/lib/libkvm/kvm_i386.c index f37da2630e1d..5aba006f047c 100644 --- a/lib/libkvm/kvm_i386.c +++ b/lib/libkvm/kvm_i386.c @@ -162,6 +162,10 @@ _i386_initvtop(kvm_t *kd) } pa = le32toh(pa); PTD = _kvm_malloc(kd, 4 * I386_PAGE_SIZE); + if (PTD == NULL) { + _kvm_err(kd, kd->program, "cannot allocate PTD"); + return (-1); + } for (i = 0; i < 4; i++) { if (kvm_read2(kd, pa + (i * sizeof(pa64)), &pa64, sizeof(pa64)) != sizeof(pa64)) { @@ -195,6 +199,10 @@ _i386_initvtop(kvm_t *kd) } pa = le32toh(pa); PTD = _kvm_malloc(kd, I386_PAGE_SIZE); + if (PTD == NULL) { + _kvm_err(kd, kd->program, "cannot allocate PTD"); + return (-1); + } if (kvm_read2(kd, pa, PTD, I386_PAGE_SIZE) != I386_PAGE_SIZE) { _kvm_err(kd, kd->program, "cannot read PTD"); return (-1); @@ -228,7 +236,7 @@ _i386_vatop(kvm_t *kd, kvaddr_t va, off_t *pa) * If we are initializing (kernel page table descriptor pointer * not yet set) then return pa == va to avoid infinite recursion. */ - if (PTD == 0) { + if (PTD == NULL) { s = _kvm_pa2off(kd, va, pa); if (s == 0) { _kvm_err(kd, kd->program, @@ -318,7 +326,7 @@ _i386_vatop_pae(kvm_t *kd, kvaddr_t va, off_t *pa) * If we are initializing (kernel page table descriptor pointer * not yet set) then return pa == va to avoid infinite recursion. */ - if (PTD == 0) { + if (PTD == NULL) { s = _kvm_pa2off(kd, va, pa); if (s == 0) { _kvm_err(kd, kd->program, diff --git a/lib/libkvm/kvm_minidump_aarch64.c b/lib/libkvm/kvm_minidump_aarch64.c index 366dd9f52ae6..d5c5197307fe 100644 --- a/lib/libkvm/kvm_minidump_aarch64.c +++ b/lib/libkvm/kvm_minidump_aarch64.c @@ -81,7 +81,7 @@ _aarch64_minidump_initvtop(kvm_t *kd) off_t off; vmst = _kvm_malloc(kd, sizeof(*vmst)); - if (vmst == 0) { + if (vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } diff --git a/lib/libkvm/kvm_minidump_amd64.c b/lib/libkvm/kvm_minidump_amd64.c index cbc132849f84..3df7a62461f0 100644 --- a/lib/libkvm/kvm_minidump_amd64.c +++ b/lib/libkvm/kvm_minidump_amd64.c @@ -81,7 +81,7 @@ _amd64_minidump_initvtop(kvm_t *kd) off_t off; vmst = _kvm_malloc(kd, sizeof(*vmst)); - if (vmst == 0) { + if (vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } diff --git a/lib/libkvm/kvm_minidump_arm.c b/lib/libkvm/kvm_minidump_arm.c index 4d7666666626..e3063ec6233a 100644 --- a/lib/libkvm/kvm_minidump_arm.c +++ b/lib/libkvm/kvm_minidump_arm.c @@ -84,7 +84,7 @@ _arm_minidump_initvtop(kvm_t *kd) off_t off; vmst = _kvm_malloc(kd, sizeof(*vmst)); - if (vmst == 0) { + if (vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } diff --git a/lib/libkvm/kvm_minidump_i386.c b/lib/libkvm/kvm_minidump_i386.c index 040d42d6931a..20871498c22d 100644 --- a/lib/libkvm/kvm_minidump_i386.c +++ b/lib/libkvm/kvm_minidump_i386.c @@ -81,7 +81,7 @@ _i386_minidump_initvtop(kvm_t *kd) off_t off; vmst = _kvm_malloc(kd, sizeof(*vmst)); - if (vmst == 0) { + if (vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } diff --git a/lib/libkvm/kvm_minidump_mips.c b/lib/libkvm/kvm_minidump_mips.c index 997b70d869b0..97b0824fa234 100644 --- a/lib/libkvm/kvm_minidump_mips.c +++ b/lib/libkvm/kvm_minidump_mips.c @@ -89,7 +89,7 @@ _mips_minidump_initvtop(kvm_t *kd) off_t off; vmst = _kvm_malloc(kd, sizeof(*vmst)); - if (vmst == 0) { + if (vmst == NULL) { _kvm_err(kd, kd->program, "cannot allocate vm"); return (-1); } diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c index 2f2b6c682c4d..1eb9872ecb5b 100644 --- a/lib/libkvm/kvm_proc.c +++ b/lib/libkvm/kvm_proc.c @@ -544,7 +544,7 @@ kvm_getprocs(kvm_t *kd, int op, int arg, int *cnt) size += size / 10; kd->procbase = (struct kinfo_proc *) _kvm_realloc(kd, kd->procbase, size); - if (kd->procbase == 0) + if (kd->procbase == NULL) return (0); osize = size; st = sysctl(mib, temp_op == KERN_PROC_ALL || @@ -614,7 +614,7 @@ kvm_getprocs(kvm_t *kd, int op, int arg, int *cnt) } size = nprocs * sizeof(struct kinfo_proc); kd->procbase = (struct kinfo_proc *)_kvm_malloc(kd, size); - if (kd->procbase == 0) + if (kd->procbase == NULL) return (0); nprocs = kvm_deadprocs(kd, op, arg, nl[1].n_value, @@ -637,21 +637,19 @@ kvm_getprocs(kvm_t *kd, int op, int arg, int *cnt) void _kvm_freeprocs(kvm_t *kd) { - if (kd->procbase) { - free(kd->procbase); - kd->procbase = 0; - } + + free(kd->procbase); + kd->procbase = NULL; } void * _kvm_realloc(kvm_t *kd, void *p, size_t n) { - void *np = (void *)realloc(p, n); + void *np; - if (np == 0) { - free(p); + np = reallocf(p, n); + if (np == NULL) _kvm_err(kd, kd->program, "out of memory"); - } return (np); } @@ -672,7 +670,7 @@ kvm_argv(kvm_t *kd, const struct kinfo_proc *kp, int env, int nchr) if (!ISALIVE(kd)) { _kvm_err(kd, kd->program, "cannot read user space from dead kernel"); - return (0); + return (NULL); } if (nchr == 0 || nchr > ARG_MAX) @@ -681,7 +679,7 @@ kvm_argv(kvm_t *kd, const struct kinfo_proc *kp, int env, int nchr) buf = malloc(nchr); if (buf == NULL) { _kvm_err(kd, kd->program, "cannot allocate memory"); - return (0); + return (NULL); } buflen = nchr; argc = 32; @@ -706,12 +704,11 @@ kvm_argv(kvm_t *kd, const struct kinfo_proc *kp, int env, int nchr) * to the requested len. */ if (errno != ENOMEM || bufsz != (size_t)buflen) - return (0); + return (NULL); buf[bufsz - 1] = '\0'; errno = 0; - } else if (bufsz == 0) { - return (0); - } + } else if (bufsz == 0) + return (NULL); i = 0; p = buf; do {