git: 811e27fa3c44 - main - jail: Add PD_KILL to remove a prison in prison_deref().

[James Gritton]

On 2021-02-24 22:02, Kyle Evans wrote:
> On Wed, Feb 24, 2021 at 11:53 PM James Gritton <jamie at freebsd.org> 
> wrote:
>> 
>> On 2021-02-24 21:29, Kyle Evans wrote:
>> > On Tue, Feb 23, 2021 at 7:16 AM Alexander Richardson
>> > <arichardson at freebsd.org> wrote:
>> >>
>> >> On Mon, 22 Feb 2021 at 20:28, Jamie Gritton <jamie at freebsd.org> wrote:
>> >> >
>> >> > The branch main has been updated by jamie:
>> >> >
>> >> > URL: https://cgit.FreeBSD.org/src/commit/?id=811e27fa3c445664e36071a7d08228fc7fb85676
>> >> >
>> >> > commit 811e27fa3c445664e36071a7d08228fc7fb85676
>> >> > Author:     Jamie Gritton <jamie at FreeBSD.org>
>> >> > AuthorDate: 2021-02-22 20:27:44 +0000
>> >> > Commit:     Jamie Gritton <jamie at FreeBSD.org>
>> >> > CommitDate: 2021-02-22 20:27:44 +0000
>> >> >
>> >> >     jail: Add PD_KILL to remove a prison in prison_deref().
>> >> >
>> >> >     Add the PD_KILL flag that instructs prison_deref() to take steps
>> >> >     to actively kill a prison and its descendents, namely marking it
>> >> >     PRISON_STATE_DYING, clearing its PR_PERSIST flag, and killing any
>> >> >     attached processes.
>> >> >
>> >> >     This replaces a similar loop in sys_jail_remove(), bringing the
>> >> >     operation under the same single hold on allprison_lock that it already
>> >> >     has. It is also used to clean up failed jail (re-)creations in
>> >> >     kern_jail_set(), which didn't generally take all the proper steps.
>> >> >
>> >> >     Differential Revision:  https://reviews.freebsd.org/D28473
>> >>
>> >> Hi Jamie,
>> >>
>> >> After this commit running cd /usr/tests/lib/libc/sys && kyua test
>> >> cpuset_test renders the entire system unusable: all exec calls
>> >> afterwards seem to fail. In Jenkins it's triggering a kernel panic:
>> >> https://ci.freebsd.org/job/FreeBSD-main-amd64-test/17630/consoleFull
>> >> Reverting this commit fixes the issue.
>> >>
>> >
>> > Based on the backtrace and a wild stab in the dark, the last
>> > prison_deref() in do_jail_attach() prior to successful return should
>> > explicitly clear the PD_KILL flag from drflags.
>> >
>> > Thanks,
>> >
>> > Kyle Evans
>> 
>> Yep, that's what's doing it.  Actually, PD_KILL has no business being
>> passed to do_jail_attach in the first place.  Running a test on that
>> right now.
>> 
> 
> Ah, good point! IMHO we should KASSERT() that as well in
> do_jail_attach(); it doesn't feel like we can do anything sensible
> with the flag there, so we might as well raise awareness that the
> caller needs to be more careful if attach failure is significant to
> the lifetime of the target.

That's a good point for safety, though I'm doing it slightly 
differently.
Now do_jail_attach() explicitly only uses the lock-status-related flags.
For the sake of redundancy, kern_jail_set() also only passes those 
flags.
I've added a couple of defines in case some other function decides to 
take
those flags: PD_OP_FLAGS for the operations that prison_deref() should 
do,
and PD_LOCK_FLAGS for the status of the prison locks.

I did add a KASSERT() in prison_deref() to detect any attempt to pass
PD_KILL to prison0.  A panic is a good deal better than a zombie system
without processes.

- Jamie