Date: Tue, 6 Oct 2015 18:07:01 +0000 (UTC) From: "Conrad E. Meyer" <cem@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r288944 - in head: lib/libprocstat lib/libutil share/man/man5 sys/kern sys/sys Message-ID: <201510061807.t96I71c9076108@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: cem Date: Tue Oct 6 18:07:00 2015 New Revision: 288944 URL: https://svnweb.freebsd.org/changeset/base/288944 Log: Fix core corruption caused by race in note_procstat_vmmap This fix is spiritually similar to r287442 and was discovered thanks to the KASSERT added in that revision. NT_PROCSTAT_VMMAP output length, when packing kinfo structs, is tied to the length of filenames corresponding to vnodes in the process' vm map via vn_fullpath. As vnodes may move during coredump, this is racy. We do not remove the race, only prevent it from causing coredump corruption. - Add a sysctl, kern.coredump_pack_vmmapinfo, to allow users to disable kinfo packing for PROCSTAT_VMMAP notes. This avoids VMMAP corruption and truncation, even if names change, at the cost of up to PATH_MAX bytes per mapped object. The new sysctl is documented in core.5. - Fix note_procstat_vmmap to self-limit in the second pass. This addresses corruption, at the cost of sometimes producing a truncated result. - Fix PROCSTAT_VMMAP consumers libutil (and libprocstat, via copy-paste) to grok the new zero padding. Reported by: pho (https://people.freebsd.org/~pho/stress/log/datamove4-2.txt) Relnotes: yes Sponsored by: EMC / Isilon Storage Division Differential Revision: https://reviews.freebsd.org/D3824 Modified: head/lib/libprocstat/libprocstat.c head/lib/libutil/kinfo_getvmmap.c head/share/man/man5/core.5 head/sys/kern/imgact_elf.c head/sys/kern/kern_exec.c head/sys/kern/kern_proc.c head/sys/sys/exec.h head/sys/sys/user.h Modified: head/lib/libprocstat/libprocstat.c ============================================================================== --- head/lib/libprocstat/libprocstat.c Tue Oct 6 17:53:29 2015 (r288943) +++ head/lib/libprocstat/libprocstat.c Tue Oct 6 18:07:00 2015 (r288944) @@ -1867,6 +1867,8 @@ kinfo_getvmmap_core(struct procstat_core eb = buf + len; while (bp < eb) { kv = (struct kinfo_vmentry *)(uintptr_t)bp; + if (kv->kve_structsize == 0) + break; bp += kv->kve_structsize; cnt++; } @@ -1882,6 +1884,8 @@ kinfo_getvmmap_core(struct procstat_core /* Pass 2: unpack */ while (bp < eb) { kv = (struct kinfo_vmentry *)(uintptr_t)bp; + if (kv->kve_structsize == 0) + break; /* Copy/expand into pre-zeroed buffer */ memcpy(kp, kv, kv->kve_structsize); /* Advance to next packed record */ Modified: head/lib/libutil/kinfo_getvmmap.c ============================================================================== --- head/lib/libutil/kinfo_getvmmap.c Tue Oct 6 17:53:29 2015 (r288943) +++ head/lib/libutil/kinfo_getvmmap.c Tue Oct 6 18:07:00 2015 (r288944) @@ -44,6 +44,8 @@ kinfo_getvmmap(pid_t pid, int *cntp) eb = buf + len; while (bp < eb) { kv = (struct kinfo_vmentry *)(uintptr_t)bp; + if (kv->kve_structsize == 0) + break; bp += kv->kve_structsize; cnt++; } @@ -59,6 +61,8 @@ kinfo_getvmmap(pid_t pid, int *cntp) /* Pass 2: unpack */ while (bp < eb) { kv = (struct kinfo_vmentry *)(uintptr_t)bp; + if (kv->kve_structsize == 0) + break; /* Copy/expand into pre-zeroed buffer */ memcpy(kp, kv, kv->kve_structsize); /* Advance to next packed record */ Modified: head/share/man/man5/core.5 ============================================================================== --- head/share/man/man5/core.5 Tue Oct 6 17:53:29 2015 (r288943) +++ head/share/man/man5/core.5 Tue Oct 6 18:07:00 2015 (r288944) @@ -28,7 +28,7 @@ .\" @(#)core.5 8.3 (Berkeley) 12/11/93 .\" $FreeBSD$ .\" -.Dd September 2, 2015 +.Dd October 5, 2015 .Dt CORE 5 .Os .Sh NAME @@ -130,6 +130,19 @@ All file descriptor information can be p This potentially wastes up to PATH_MAX bytes per open fd. Packing is disabled with .Dl sysctl kern.coredump_pack_fileinfo=0 . +.Pp +Similarly, corefiles are written with vmmap information as an ELF note, which +contains file paths. +By default, they are packed to only use as much space as +needed. +By the same mechanism as for the open files note, these paths can also +change at any time and result in a truncated note. +.Pp +All vmmap information can be preserved by disabling packing. +Like the file information, this potentially wastes up to PATH_MAX bytes per +mapped object. +Packing is disabled with +.Dl sysctl kern.coredump_pack_vmmapinfo=0 . .Sh EXAMPLES In order to store all core images in per-user private areas under .Pa /var/coredumps , Modified: head/sys/kern/imgact_elf.c ============================================================================== --- head/sys/kern/imgact_elf.c Tue Oct 6 17:53:29 2015 (r288943) +++ head/sys/kern/imgact_elf.c Tue Oct 6 18:07:00 2015 (r288944) @@ -1959,24 +1959,30 @@ note_procstat_vmmap(void *arg, struct sb { struct proc *p; size_t size; - int structsize; + int structsize, vmmap_flags; + + if (coredump_pack_vmmapinfo) + vmmap_flags = KERN_VMMAP_PACK_KINFO; + else + vmmap_flags = 0; p = (struct proc *)arg; + structsize = sizeof(struct kinfo_vmentry); if (sb == NULL) { size = 0; sb = sbuf_new(NULL, NULL, 128, SBUF_FIXEDLEN); sbuf_set_drain(sb, sbuf_drain_count, &size); sbuf_bcat(sb, &structsize, sizeof(structsize)); PROC_LOCK(p); - kern_proc_vmmap_out(p, sb); + kern_proc_vmmap_out(p, sb, -1, vmmap_flags); sbuf_finish(sb); sbuf_delete(sb); *sizep = size; } else { - structsize = sizeof(struct kinfo_vmentry); sbuf_bcat(sb, &structsize, sizeof(structsize)); PROC_LOCK(p); - kern_proc_vmmap_out(p, sb); + kern_proc_vmmap_out(p, sb, *sizep - sizeof(structsize), + vmmap_flags); } } Modified: head/sys/kern/kern_exec.c ============================================================================== --- head/sys/kern/kern_exec.c Tue Oct 6 17:53:29 2015 (r288943) +++ head/sys/kern/kern_exec.c Tue Oct 6 18:07:00 2015 (r288944) @@ -105,6 +105,11 @@ SYSCTL_INT(_kern, OID_AUTO, coredump_pac &coredump_pack_fileinfo, 0, "Enable file path packing in 'procstat -f' coredump notes"); +int coredump_pack_vmmapinfo = 1; +SYSCTL_INT(_kern, OID_AUTO, coredump_pack_vmmapinfo, CTLFLAG_RWTUN, + &coredump_pack_vmmapinfo, 0, + "Enable file path packing in 'procstat -v' coredump notes"); + static int sysctl_kern_ps_strings(SYSCTL_HANDLER_ARGS); static int sysctl_kern_usrstack(SYSCTL_HANDLER_ARGS); static int sysctl_kern_stackprot(SYSCTL_HANDLER_ARGS); Modified: head/sys/kern/kern_proc.c ============================================================================== --- head/sys/kern/kern_proc.c Tue Oct 6 17:53:29 2015 (r288943) +++ head/sys/kern/kern_proc.c Tue Oct 6 18:07:00 2015 (r288944) @@ -2252,7 +2252,7 @@ next:; * Must be called with the process locked and will return unlocked. */ int -kern_proc_vmmap_out(struct proc *p, struct sbuf *sb) +kern_proc_vmmap_out(struct proc *p, struct sbuf *sb, ssize_t maxlen, int flags) { vm_map_entry_t entry, tmp_entry; struct vattr va; @@ -2276,7 +2276,7 @@ kern_proc_vmmap_out(struct proc *p, stru PRELE(p); return (ESRCH); } - kve = malloc(sizeof(*kve), M_TEMP, M_WAITOK); + kve = malloc(sizeof(*kve), M_TEMP, M_WAITOK | M_ZERO); error = 0; map = &vm->vm_map; @@ -2411,10 +2411,23 @@ kern_proc_vmmap_out(struct proc *p, stru free(freepath, M_TEMP); /* Pack record size down */ - kve->kve_structsize = offsetof(struct kinfo_vmentry, kve_path) + - strlen(kve->kve_path) + 1; + if ((flags & KERN_VMMAP_PACK_KINFO) != 0) + kve->kve_structsize = + offsetof(struct kinfo_vmentry, kve_path) + + strlen(kve->kve_path) + 1; + else + kve->kve_structsize = sizeof(*kve); kve->kve_structsize = roundup(kve->kve_structsize, sizeof(uint64_t)); + + /* Halt filling and truncate rather than exceeding maxlen */ + if (maxlen != -1 && maxlen < kve->kve_structsize) { + error = 0; + vm_map_lock_read(map); + break; + } else if (maxlen != -1) + maxlen -= kve->kve_structsize; + if (sbuf_bcat(sb, kve, kve->kve_structsize) != 0) error = ENOMEM; vm_map_lock_read(map); @@ -2447,7 +2460,7 @@ sysctl_kern_proc_vmmap(SYSCTL_HANDLER_AR sbuf_delete(&sb); return (error); } - error = kern_proc_vmmap_out(p, &sb); + error = kern_proc_vmmap_out(p, &sb, -1, KERN_VMMAP_PACK_KINFO); error2 = sbuf_finish(&sb); sbuf_delete(&sb); return (error != 0 ? error : error2); Modified: head/sys/sys/exec.h ============================================================================== --- head/sys/sys/exec.h Tue Oct 6 17:53:29 2015 (r288943) +++ head/sys/sys/exec.h Tue Oct 6 18:07:00 2015 (r288944) @@ -84,6 +84,7 @@ int exec_register(const struct execsw *) int exec_unregister(const struct execsw *); extern int coredump_pack_fileinfo; +extern int coredump_pack_vmmapinfo; /* * note: name##_mod cannot be const storage because the Modified: head/sys/sys/user.h ============================================================================== --- head/sys/sys/user.h Tue Oct 6 17:53:29 2015 (r288943) +++ head/sys/sys/user.h Tue Oct 6 18:07:00 2015 (r288944) @@ -541,6 +541,9 @@ struct kinfo_sigtramp { /* Flags for kern_proc_filedesc_out. */ #define KERN_FILEDESC_PACK_KINFO 0x00000001U + +/* Flags for kern_proc_vmmap_out. */ +#define KERN_VMMAP_PACK_KINFO 0x00000001U struct sbuf; /* @@ -556,7 +559,8 @@ int kern_proc_filedesc_out(struct proc * int flags); int kern_proc_cwd_out(struct proc *p, struct sbuf *sb, ssize_t maxlen); int kern_proc_out(struct proc *p, struct sbuf *sb, int flags); -int kern_proc_vmmap_out(struct proc *p, struct sbuf *sb); +int kern_proc_vmmap_out(struct proc *p, struct sbuf *sb, ssize_t maxlen, + int flags); int vntype_to_kinfo(int vtype); #endif /* !_KERNEL */
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201510061807.t96I71c9076108>