From owner-freebsd-hackers@FreeBSD.ORG Mon Sep 29 19:28:08 2003 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2EA4216A4B3 for ; Mon, 29 Sep 2003 19:28:08 -0700 (PDT) Received: from afields.ca (afields.ca [216.194.67.132]) by mx1.FreeBSD.org (Postfix) with ESMTP id E8C1643F75 for ; Mon, 29 Sep 2003 19:28:06 -0700 (PDT) (envelope-from afields@afields.ca) Received: from afields.ca (localhost.afields.ca [127.0.0.1]) by afields.ca (8.12.6/8.12.6) with ESMTP id h8U2S5PD025147; Mon, 29 Sep 2003 22:28:05 -0400 (EDT) (envelope-from afields@afields.ca) Received: (from afields@localhost) by afields.ca (8.12.6/8.12.6/Submit) id h8U2S5NJ025146; Mon, 29 Sep 2003 22:28:05 -0400 (EDT) (envelope-from afields) Date: Mon, 29 Sep 2003 22:28:05 -0400 From: Allan Fields To: freebsd-hackers@freebsd.org Message-ID: <20030930022804.GB94150@afields.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.4i Subject: ps,libkvm,ls malloc/free X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Allan Fields List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Sep 2003 02:28:08 -0000 Hi, While trying to debug issues with libkvm access from an experimental tool (quick-hack) I've been working on, I've come across a segfault that occurs in _kvm_malloc(). This is a stock install of FreeBSD 5.1-RELEASE but the code invoking libkvm is originally taken from bin/ps/ps.c in the latest RELENG_5_1 source distribution. The situation is related to a call of kvm_openfiles() subsequent to a previous call and kvm_close(). I've poked around in the most recent libkvm sources and so-far didn't see why _kvm_malloc() would fail in this case. Further my backtrace doesn't show the same calling order one might expect: Program terminated with signal 11, Segmentation fault. #0 0x080ea74d in _kvm_malloc () #1 0x080eaa41 in kvm_openfiles () #2 0x08066e3a in main (argc=1, argv=0x82f4000) at /usr/src-5_1/bin/ps/ps.c:324 In the source it looks more like: kvm_openfiles() => _kvm_open() (and I can't find the _kvm_malloc() call.) Does this have anything to do with changes for descriptors being flagged close-on-exec? Has something changed recently so that I have to update libkvm? I solved a similar issue with fts_open() segfaulting bin/ls, on subsequent calls by cleaning up before traverse() returns. [Patch below.] To be truthful: it is my non-standard usage of these routines that has turned up these problems, and I was expecting some issues like this to appear. This problem is appearing only because of the atypical usage of these calls multiple times with-in the same process which could potentially be avoided. But my question is: why doesn't kvm_close() do the job? Is kvm_open*() allowed to be called more than once by the same process? I also noticed that certain binaries including ls and ps don't explicitly free memory and close file descriptors before return. Usually this is not a problem, however: is this the right approach to take from the standpoint of code style? Is it fair to assume a return from main() will always clean-up for us? I have made this assumption myself before. exit(3) usually does what we need. I can see that it wouldn't be hard to go through and create #ifdef CLEANUP blocks to give the option of explicit clean-up. Better still would be avoiding situations where there are items remaining to be free()ed on return by cleaning up directly after use. Also of concern to me here is the number of globals in the base system commands, but so far this hasn't been a serious problem. I'm actually suprised I have had the sucess with the approach that I have. -- Allan Fields ----- diff -u ls.c.orig ls.c: --- ls.c.orig Mon Sep 29 06:10:41 2003 +++ ls.c Mon Sep 29 06:39:28 2003 @@ -507,6 +507,12 @@ } if (errno) err(1, "fts_read"); + + /* Clean-up: */ + free(p); + if (fts_close(ftsp)) /* explicitly close descriptor */ + err(1, "fts_close"); + } /* -----