From owner-freebsd-hackers@FreeBSD.ORG Wed Oct 3 16:57:56 2012 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D88C7106566C for ; Wed, 3 Oct 2012 16:57:56 +0000 (UTC) (envelope-from avg@FreeBSD.org) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 3642B8FC0C for ; Wed, 3 Oct 2012 16:57:55 +0000 (UTC) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id TAA08280 for ; Wed, 03 Oct 2012 19:57:54 +0300 (EEST) (envelope-from avg@FreeBSD.org) Message-ID: <506C6E92.10803@FreeBSD.org> Date: Wed, 03 Oct 2012 19:57:54 +0300 From: Andriy Gapon User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 MIME-Version: 1.0 To: freebsd-hackers X-Enigmail-Version: 1.4.3 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: Subject: kvm_getprocs: gracefully handle errors in kvm_deadprocs X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Oct 2012 16:57:56 -0000 kvm_deadprocs returns -1 to signify an error. Current kvm_getprocs code would pass this return code as 'cnt' out parameter and would not reset return value to NULL. This confuses some callers, most prominently procstat_getprocs, into believing that kvm_getprocs was successful. Moreover, the code tried to use cnt=-1 as an unsigned number to allocate some additional memory. As a result fstat -M could try to allocate huge amount of memory e.g. when used with a kernel that didn't match userland. With the proposed change the error code should be handled properly. Additionally it should now be possible to enable realloc code, which previously contained a bug and was called even after kvm_deadprocs error. commit 6ddf602409119eded40321e5bb349b464f24e81a Author: Andriy Gapon Date: Sun Sep 23 22:52:28 2012 +0300 kvm_proc: gracefully handle errors in kvm_deadprocs, don't confuse callers Plus fix a bug under 'notdef' (sic) section. diff --git a/lib/libkvm/kvm_proc.c b/lib/libkvm/kvm_proc.c index d1daf77..31258d7 100644 --- a/lib/libkvm/kvm_proc.c +++ b/lib/libkvm/kvm_proc.c @@ -593,9 +593,15 @@ liveout: nprocs = kvm_deadprocs(kd, op, arg, nl[1].n_value, nl[2].n_value, nprocs); + if (nprocs <= 0) { + _kvm_freeprocs(kd); + nprocs = 0; + } #ifdef notdef - size = nprocs * sizeof(struct kinfo_proc); - (void)realloc(kd->procbase, size); + else { + size = nprocs * sizeof(struct kinfo_proc); + kd->procbase = realloc(kd->procbase, size); + } #endif } *cnt = nprocs; P.S. it might may sense to change 'count' parameter of procstat_getprocs to signed int so that it matches kvm_getprocs interface. Alternatively, an intermediate variable could be used to insulate signedness difference: index 56562e1..11a817e 100644 --- a/lib/libprocstat/libprocstat.c +++ b/lib/libprocstat/libprocstat.c @@ -184,15 +184,18 @@ procstat_getprocs(struct procstat *procstat, int what, int arg, struct kinfo_proc *p0, *p; size_t len; int name[4]; + int cnt; int error; assert(procstat); assert(count); p = NULL; if (procstat->type == PROCSTAT_KVM) { - p0 = kvm_getprocs(procstat->kd, what, arg, count); - if (p0 == NULL || count == 0) + *count = 0; + p0 = kvm_getprocs(procstat->kd, what, arg, &cnt); + if (p0 == NULL || cnt <= 0) return (NULL); + *count = cnt; len = *count * sizeof(*p); p = malloc(len); if (p == NULL) { -- Andriy Gapon