Date: Sat, 5 Sep 2015 16:04:25 +0200 From: Tijl Coosemans <tijl@FreeBSD.org> To: "Conrad E. Meyer" <cem@FreeBSD.org> 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>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 3 Sep 2015 20:32:10 +0000 (UTC) "Conrad E. Meyer" <cem@FreeBSD.org> 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".
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150905160425.2b7c4088>