On Sun, 14 Jul 2019, [UTF-8] ?~N~K?~X~J?~D? wrote:

Thanks for share this. The followings are my suggestions.


But debugging also
requires normal signal handling, perhaps including core dumps, so
my replacements for the bogus pid tests may be too strong.
I think this could be fixed by allowing default actions be taken during
debugging of init(8), in kern/kern_sig.c:

I now prefer a more conservative change at first:
- keep P_SYSTEM for init
- check that P_SYSTEM is set for all pids <= 1, and remove the redundant
  pid tests in places that now check both pid <= 1 and P_SYSTEM
- open small holes in P_SYSTEM checks allow useful things that are believed
  to be safe.  Mainly ptrace on init.
- add P_SYSTEM checks for p_candebug() and p_cansignal()...
- open small hoes in the new P_SYSTEM checks.

In kern, p_candebug() is currently required for ktr, pget() and ptrace().
I think ktr doesn't operate on init, and we want to allow pget() and
ptrace(), except at higher securelevels.  This gets complicated.
securelevels used to be documented only in init(8), and init is still
responsible for managing them in some cases.  init can also be run in
jails, and there is a per-jail securelevel.  I don't really understand
jails, but think that we don't want to make it too easy to run ptrace
on init in jails.  Hopefully the per-jail securelevel is enough to
control this.

p_candebug() actually has always had a check that disallows tracing of
specifically initproc at securelevel > 0.  This check was not in 4.4BSD.
It was added to sys_process.c in 1997 (r25200) and had no effect there
until 2000 (r65237 (p_can* reorganization)) when it was have no effect
in kern_prot.c.  So nothing new needs to be done for securelevels provided
this code is correct.

Don't take default actions on system processes.
Change to something like:

/*
* Don't take default actions on system processes.
*/
if ((p == initproc && !(p->p_flag & P_TRACED)) ||
   p->p_flag & P_SYSTEM) {

This is essentially the method of opening a hole for init (we open the
hole which allows P_TRACED to be set for init elsewhere, and here we trust
P_TRACED to only be set if default signal actions are allowed).  This is
a bit too strong.  I don't want P_TRACED to allow default SIGKILL actions.
ptrace() can't control those.  It can control some other signals whose
default is to kill the process.

- kern_proc.c:stop_all_proc(): to disallow stopping system processes.
   This seems right for init, and the change breaks it.
Add an additional check for init(8) seems needed, along with the 'P_SYSTEM'
checking.

Keeping P_SYSTEM for init preserves existing behaviour here and in any other
cases that we haven't noticed.

Bruce
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"

Reply via email to