Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 5 Sep 2015 07:50:13 -0700
From:      Conrad Meyer <cem@FreeBSD.org>
To:        Tijl Coosemans <tijl@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:  <CAG6CVpXOEVvvMG7HgK732%2B_K-yYMjSxWNzCoNPrejD2XC60AKQ@mail.gmail.com>
In-Reply-To: <20150905160425.2b7c4088@kalimero.tijl.coosemans.org>
References:  <201509032032.t83KWAtl043698@repo.freebsd.org> <20150905160425.2b7c4088@kalimero.tijl.coosemans.org>

next in thread | previous in thread | raw e-mail | index | archive | help
I'll be offline for the next ~48 hours. It sounds like it's annoying
but not critical to fix. If it isn't fixed when I get back, I'll
figure something out.

Thanks,
Conrad

P.S., What systems have both imgact_elf64 and 32?

On Sat, Sep 5, 2015 at 7:04 AM, Tijl Coosemans <tijl@freebsd.org> wrote:
> 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?CAG6CVpXOEVvvMG7HgK732%2B_K-yYMjSxWNzCoNPrejD2XC60AKQ>