Details

Reviewers

jhb
jilles

Commits

rG5844bd058aed: jobc: rework detection of orphaned groups.
rGcf4f802e77a3: kinfo_proc: move job-control related data collection into a new helper.
rG4daea938130b: Lock proctree in around fill_kinfo_proc().
rGa008bdeda3b8: tty_wait_background: improve locking.
rGef739c7373d8: pgrp: Prevent use after free.
rGe0d83cd3e49f: issignal(): when handling STOP-like signals, drop sigacts mutex earlier.
rG993a1699b194: Style. Improve some KASSERTs messages.

Summary

Instead of trying to maintain pg_jobc counter on each process group update (and sometimes before), just calculate the counter when needed. Still, for the benefit of the signal delivery code, explicitly mark orphaned groups as such with the new process group flag.

Fix use after free when relocking p_pgrp in tty_wait_background (and probably other places). Change pgrp memory to type-stable zone, and ensure that pg_mtx is not destroyed.

Not sure how to present it in phab, there https://kib.kiev.ua/git/gitweb.cgi?p=deviant3.git;a=shortlog;h=refs/heads/jobcdbg is the branch with individual commits.

Tested by: pho

Test Plan

Shells work fine.
I checked things like

# sh
# cat &
# ^D

to deliver expected signals to the stopped orphaned processes.

Also there is tools/regression/sigqueue/sigqtest2

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

kib requested review of this revision.Dec 31 2020, 7:10 PM

kib created this revision.

kib mentioned this in D27816: jobc: correct check for ignored children..Dec 31 2020, 7:13 PM

I got a watchdog timeout with this patch: https://people.freebsd.org/~pho/stress/log/log0013.txt

Fix LOR between proctree and pidhash bucket lock.
Do not forget un-orphaning groups when reparenting on exit.

FWIW, I upload patch series as a set of arc reviews. Originally this was tedious (using arc diff --create --head <N> <N-1> for each commit), but @markj has been working on a script that automates this (I used it to upload the series of zombproc-related debugger cleanups as a single command from a branch with multiple commits).

sys/kern/kern_proc.c
1352

20210101 07:17:42 all (6/740): machipc2.sh
panic: Lock (sx) proctree not locked @ kern/kern_proc.c:1172.
cpuid = 17
time = 1609481864
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe01c7c16ab0
vpanic() at vpanic+0x181/frame 0xfffffe01c7c16b00
panic() at panic+0x43/frame 0xfffffe01c7c16b60
witness_assert() at witness_assert+0x23a/frame 0xfffffe01c7c16b90
fill_kinfo_proc() at fill_kinfo_proc+0x4e/frame 0xfffffe01c7c16be0
kern_proc_out() at kern_proc_out+0x62/frame 0xfffffe01c7c17370
sysctl_out_proc() at sysctl_out_proc+0x68/frame 0xfffffe01c7c17840
sysctl_kern_proc() at sysctl_kern_proc+0x87/frame 0xfffffe01c7c17890
sysctl_root_handler_locked() at sysctl_root_handler_locked+0x9c/frame 0xfffffe01c7c178e0
sysctl_root() at sysctl_root+0x20d/frame 0xfffffe01c7c17960
userland_sysctl() at userland_sysctl+0x180/frame 0xfffffe01c7c17a10
sys___sysctl() at sys___sysctl+0x5f/frame 0xfffffe01c7c17ac0
amd64_syscall() at amd64_syscall+0x147/frame 0xfffffe01c7c17bf0
fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe01c7c17bf0
--- syscall (202, FreeBSD ELF64, sys___sysctl), rip = 0x8003fb9ea, rsp = 0x7fffffffd258, rbp = 0x7fffffffd290 ---
KDB: enter: panic
[ thread pid 78475 tid 792240 ]
Stopped at      kdb_enter+0x37: movq    $0,0x10960c6(%rip)
db>

https://people.freebsd.org/~pho/stress/log/log0014.txt

One more place for proctree_lock.

kinfo_proc_out is also called from ELF coredump code

kib edited the summary of this revision. (Show Details)Jan 4 2021, 7:09 PM

kib added a reviewer: jilles.Jan 4 2021, 10:04 PM

jilles accepted this revision.Jan 9 2021, 7:50 PM

This revision is now accepted and ready to land.Jan 9 2021, 7:50 PM

Closed by commit rG993a1699b194: Style. Improve some KASSERTs messages. (authored by kib). · Explain WhyJan 10 2021, 2:42 AM

This revision was automatically updated to reflect the committed changes.

kib added a commit: rG993a1699b194: Style. Improve some KASSERTs messages..

kib added a commit: rGe0d83cd3e49f: issignal(): when handling STOP-like signals, drop sigacts mutex earlier..

kib added a commit: rGef739c7373d8: pgrp: Prevent use after free..