From owner-svn-src-all@freebsd.org Sat Sep 5 14:04:51 2015 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id C2A0A9CA9E4; Sat, 5 Sep 2015 14:04:51 +0000 (UTC) (envelope-from tijl@freebsd.org) Received: from mailrelay111.isp.belgacom.be (mailrelay111.isp.belgacom.be [195.238.20.138]) by mx1.freebsd.org (Postfix) with ESMTP id BBEE9B9; Sat, 5 Sep 2015 14:04:50 +0000 (UTC) (envelope-from tijl@freebsd.org) X-Belgacom-Dynamic: yes X-Cloudmark-SP-Filtered: true X-Cloudmark-SP-Result: v=1.1 cv=z2WTV4mAI6UWamCjDJCdLIA+c/M+fkeu8SyhGDYh7nk= c=1 sm=2 a=6I5d2MoRAAAA:8 a=pL__uFuU6VXqykJWFi8A:9 a=CjuIK1q_8ugA:10 a=pEm6xrANGHqmbCiX:21 a=KcrZtCmqaCGjBTt7:21 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: A2CaBgDr9epV/4OL8VFdgyNUWg+/cYV5AoEkPRABAQEBAQEBgQqEJAEBBDocIxALFAQJJQ8qHgYTiDIBCMoBAQEBAQEBAQMBAQEBAQEBFwSLboQ0DxYzB4QsAQSVVYUKh21UmiYmghAcFoFAPDOIA4FHAQEB Received: from 131.139-241-81.adsl-dyn.isp.belgacom.be (HELO kalimero.tijl.coosemans.org) ([81.241.139.131]) by relay.skynet.be with ESMTP; 05 Sep 2015 16:04:28 +0200 Received: from kalimero.tijl.coosemans.org (kalimero.tijl.coosemans.org [127.0.0.1]) by kalimero.tijl.coosemans.org (8.15.2/8.15.2) with ESMTP id t85E4Ptg002203; Sat, 5 Sep 2015 16:04:26 +0200 (CEST) (envelope-from tijl@FreeBSD.org) Date: Sat, 5 Sep 2015 16:04:25 +0200 From: Tijl Coosemans To: "Conrad E. Meyer" Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r287442 - in head: lib/libprocstat lib/libutil share/man/man5 sys/kern sys/sys Message-ID: <20150905160425.2b7c4088@kalimero.tijl.coosemans.org> In-Reply-To: <201509032032.t83KWAtl043698@repo.freebsd.org> References: <201509032032.t83KWAtl043698@repo.freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 05 Sep 2015 14:04:52 -0000 On Thu, 3 Sep 2015 20:32:10 +0000 (UTC) "Conrad E. Meyer" wrote: > Author: cem > Date: Thu Sep 3 20:32:10 2015 > New Revision: 287442 > URL: https://svnweb.freebsd.org/changeset/base/287442 > > Log: > Detect badly behaved coredump note helpers > > Coredump notes depend on being able to invoke dump routines twice; once > in a dry-run mode to get the size of the note, and another to actually > emit the note to the corefile. > > When a note helper emits a different length section the second time > around than the length it requested the first time, the kernel produces > a corrupt coredump. > > NT_PROCSTAT_FILES output length, when packing kinfo structs, is tied to > the length of filenames corresponding to vnodes in the process' fd table > via vn_fullpath. As vnodes may move around during dump, this is racy. > > So: > > - Detect badly behaved notes in putnote() and pad underfilled notes. > > - Add a fail point, debug.fail_point.fill_kinfo_vnode__random_path to > exercise the NT_PROCSTAT_FILES corruption. It simply picks random > lengths to expand or truncate paths to in fo_fill_kinfo_vnode(). > > - Add a sysctl, kern.coredump_pack_fileinfo, to allow users to > disable kinfo packing for PROCSTAT_FILES notes. This should avoid > both FILES note corruption and truncation, even if filenames change, > at the cost of about 1 kiB in padding bloat per open fd. Document > the new sysctl in core.5. > > - Fix note_procstat_files to self-limit in the 2nd pass. Since > sometimes this will result in a short write, pad up to our advertised > size. This addresses note corruption, at the risk of sometimes > truncating the last several fd info entries. > > - Fix NT_PROCSTAT_FILES consumers libutil and libprocstat to grok the > zero padding. > > With suggestions from: bjk, jhb, kib, wblock > Approved by: markj (mentor) > Relnotes: yes > Sponsored by: EMC / Isilon Storage Division > Differential Revision: https://reviews.freebsd.org/D3548 > > Modified: > head/lib/libprocstat/libprocstat.c > head/lib/libutil/kinfo_getfile.c > head/share/man/man5/core.5 > head/sys/kern/imgact_elf.c > head/sys/kern/kern_descrip.c > head/sys/kern/vfs_vnops.c > head/sys/sys/user.h > > Modified: head/sys/kern/imgact_elf.c > ============================================================================== > --- head/sys/kern/imgact_elf.c Thu Sep 3 19:42:56 2015 (r287441) > +++ head/sys/kern/imgact_elf.c Thu Sep 3 20:32:10 2015 (r287442) > @@ -1875,29 +1902,56 @@ __elfN(note_procstat_proc)(void *arg, st > CTASSERT(sizeof(struct kinfo_file) == KINFO_FILE_SIZE); > #endif > > +static int pack_fileinfo = 1; > +SYSCTL_INT(_kern, OID_AUTO, coredump_pack_fileinfo, CTLFLAG_RWTUN, > + &pack_fileinfo, 0, > + "Enable file path packing in 'procstat -f' coredump notes"); This file can be compiled twice (included by both imgact_elf32.c and imgact_elf64.c) so this sysctl can be added twice and the second time results in an error message in dmesg: "can't re-use a leaf".