Bug 204999

Summary:	Workaround for premature kthread exit is wrong
Product:	Base System	Reporter:	John Baldwin <jhb>
Component:	kern	Assignee:	John Baldwin <jhb>
Status:	Closed FIXED
Severity:	Affects Only Me	CC:	kai, kib, op
Priority:	---	Flags:	op: mfc-stable10?
Version:	10.2-STABLE
Hardware:	Any
OS:	Any

Description John Baldwin freebsd_committer

2015-12-03 19:13:29 UTC

If a kthread returns instead of calling kthread_exit(), fork_exit() tries to cope by exiting for it.  However, it calls kproc_exit() instead of kthread_exit().  This is especially disastrous if you fork a kthread from proc 0 as killing proc 0 breaks all sorts of things.

Today I was debugging a device driver that spawned a kthread from proc0 that returned.  It resulted in a double fault of nested traps from the fork_exit as well as triggered a double fault in an idle thread.  I think that the kproc_exit(0) on proc0 just broke a lot of things.  The fix is to change this to kthread_exit().

This bug is mostly a reminder so I don't forget to write up a test case and test the fix.

Comment 1 John Baldwin freebsd_committer

2016-02-08 22:32:03 UTC

Another fun fact: the test in fork_exit() checks P_KTHREAD, so it doesn't handle threads in proc0 at all.  Instead, those threads try to return to userland via a garbage trapframe resulting in hangs or odd panics.

Comment 2 commit-hook freebsd_committer

2016-02-08 23:06:46 UTC

A commit references this bug:

Author: jhb
Date: Mon Feb  8 23:06:27 UTC 2016
New revision: 295418
URL: https://svnweb.freebsd.org/changeset/base/295418

Log:
  Mark proc0 as a kernel process via the P_KTHREAD flag.

  All other kernel processes have this flag set and all threads in proc0
  (including thread0) have the similar TDP_KTHREAD flag set.

  PR:		204999
  Submitted by:	Oliver Pinter @ HardenedBSD
  Reviewed by:	kib
  MFC after:	1 week

Changes:
  head/sys/kern/init_main.c

Comment 3 commit-hook freebsd_committer

2016-02-08 23:11:48 UTC

A commit references this bug:

Author: jhb
Date: Mon Feb  8 23:11:23 UTC 2016
New revision: 295419
URL: https://svnweb.freebsd.org/changeset/base/295419

Log:
  Call kthread_exit() rather than kproc_exit() for a premature kthread exit.

  Kernel threads (and processes) are supposed to call kthread_exit() (or
  kproc_exit()) to terminate.  However, the kernel includes a fallback in
  fork_exit() to force a kthread exit if a kernel thread's "main" routine
  returns.  This fallback was added back when the kernel only had processes
  and was not updated to call kthread_exit() instead of kproc_exit() when
  threads were added to the kernel.

  This mistake was particular exciting when the errant thread belonged to
  proc0.  Due to the missing P_KTHREAD flag the fallback did not kick in
  and instead tried to return to userland via whatever garbage was in the
  trapframe.  With P_KTHREAD set it tried to terminate proc0 resulting in
  other amusements.

  PR:		204999
  MFC after:	1 week

Changes:
  head/sys/kern/kern_fork.c

Comment 4 commit-hook freebsd_committer

2016-02-16 21:37:28 UTC

A commit references this bug:

Author: jhb
Date: Tue Feb 16 21:36:49 UTC 2016
New revision: 295674
URL: https://svnweb.freebsd.org/changeset/base/295674

Log:
  MFC 295418,295419:
  Fix hangs or panics when misbehaved kernel threads return from their
  main function.

  295418:
  Mark proc0 as a kernel process via the P_KTHREAD flag.

  All other kernel processes have this flag set and all threads in proc0
  (including thread0) have the similar TDP_KTHREAD flag set.

  295419:
  Call kthread_exit() rather than kproc_exit() for a premature kthread exit.

  Kernel threads (and processes) are supposed to call kthread_exit() (or
  kproc_exit()) to terminate.  However, the kernel includes a fallback in
  fork_exit() to force a kthread exit if a kernel thread's "main" routine
  returns.  This fallback was added back when the kernel only had processes
  and was not updated to call kthread_exit() instead of kproc_exit() when
  threads were added to the kernel.

  This mistake was particularly exciting when the errant thread belonged to
  proc0.  Due to the missing P_KTHREAD flag the fallback did not kick in
  and instead tried to return to userland via whatever garbage was in the
  trapframe.  With P_KTHREAD set it tried to terminate proc0 resulting in
  other amusements.

  PR:		204999
  Approved by:	re (glebius)

Changes:
_U  stable/10/
  stable/10/sys/kern/init_main.c
  stable/10/sys/kern/kern_fork.c

Comment 5 commit-hook freebsd_committer

2016-02-17 01:45:47 UTC

A commit references this bug:

Author: jhb
Date: Wed Feb 17 01:45:35 UTC 2016
New revision: 295684
URL: https://svnweb.freebsd.org/changeset/base/295684

Log:
  MFC 295418,295419:
  Fix hangs or panics when misbehaved kernel threads return from their
  main function.

  295418:
  Mark proc0 as a kernel process via the P_KTHREAD flag.

  All other kernel processes have this flag set and all threads in proc0
  (including thread0) have the similar TDP_KTHREAD flag set.

  295419:
  Call kthread_exit() rather than kproc_exit() for a premature kthread exit.

  Kernel threads (and processes) are supposed to call kthread_exit() (or
  kproc_exit()) to terminate.  However, the kernel includes a fallback in
  fork_exit() to force a kthread exit if a kernel thread's "main" routine
  returns.  This fallback was added back when the kernel only had processes
  and was not updated to call kthread_exit() instead of kproc_exit() when
  threads were added to the kernel.

  This mistake was particularly exciting when the errant thread belonged to
  proc0.  Due to the missing P_KTHREAD flag the fallback did not kick in
  and instead tried to return to userland via whatever garbage was in the
  trapframe.  With P_KTHREAD set it tried to terminate proc0 resulting in
  other amusements.

  PR:		204999

Changes:
_U  stable/8/sys/
_U  stable/8/sys/kern/
  stable/8/sys/kern/init_main.c
  stable/8/sys/kern/kern_fork.c
_U  stable/9/sys/
  stable/9/sys/kern/init_main.c
  stable/9/sys/kern/kern_fork.c

Comment 6 commit-hook freebsd_committer

2019-10-02 14:54:29 UTC

A commit references this bug:

Author: kai
Date: Wed Oct  2 14:53:52 UTC 2019
New revision: 513582
URL: https://svnweb.freebsd.org/changeset/ports/513582

Log:
  net-mgmt/cacti: Update to 1.2.7

  Changelog since: 1.2.5:

  * Fixes CVE-2019-16723 that allowed unrestricted access to graphs
  * Various bugfixes

  https://github.com/Cacti/cacti/blob/release/1.2.7/CHANGELOG

  PR:		204999
  Submitted by:	Michael Muenz <m.muenz@gmail.com>
  Approved by:	Daniel Austin <freebsd-ports@dan.me.uk> (maintainer)
  MFH:		2019Q4
  Security:	ed18aa92-e4f4-11e9-b6fa-3085a9a95629

Changes:
  head/net-mgmt/cacti/Makefile
  head/net-mgmt/cacti/distinfo
  head/net-mgmt/cacti/files/patch-lib_installer.php
  head/net-mgmt/cacti/pkg-plist

Comment 7 commit-hook freebsd_committer

2019-10-03 10:37:20 UTC

A commit references this bug:

Author: kai
Date: Thu Oct  3 10:36:56 UTC 2019
New revision: 513661
URL: https://svnweb.freebsd.org/changeset/ports/513661

Log:
  MFH: r513582

  net-mgmt/cacti: Update to 1.2.7

  Changelog since: 1.2.5:

  * Fixes CVE-2019-16723 that allowed unrestricted access to graphs
  * Various bugfixes

  https://github.com/Cacti/cacti/blob/release/1.2.7/CHANGELOG

  PR:		204999
  Submitted by:	Michael Muenz <m.muenz@gmail.com>
  Approved by:	Daniel Austin <freebsd-ports@dan.me.uk> (maintainer)
  Security:	ed18aa92-e4f4-11e9-b6fa-3085a9a95629

  Approved by:	ports-secteam (miwi)

Changes:
_U  branches/2019Q4/
  branches/2019Q4/net-mgmt/cacti/Makefile
  branches/2019Q4/net-mgmt/cacti/distinfo
  branches/2019Q4/net-mgmt/cacti/files/patch-lib_installer.php
  branches/2019Q4/net-mgmt/cacti/pkg-plist

Comment 8 Kai Knoblich freebsd_committer

2019-10-05 21:54:00 UTC

Sorry for noise here: There was a typo in my both commits related to bug #240999 so the commit notifications went in this PR.