Date: Thu, 25 Feb 2021 20:10:01 -0800 From: James Gritton <jamie@freebsd.org> To: Kyle Evans <kevans@freebsd.org> Cc: Alexander Richardson <arichardson@freebsd.org>, src-committers <src-committers@freebsd.org>, <dev-commits-src-all@freebsd.org>, dev-commits-src-main@freebsd.org Subject: Re: git: 811e27fa3c44 - main - jail: Add PD_KILL to remove a prison in prison_deref(). Message-ID: <6effcf7f64ec5efec1c275d4c7798017@freebsd.org> In-Reply-To: <CACNAnaE4XaAR-S9scg4iG9FOa9b1dHjsuh8ocUpN=NqEip6cKA@mail.gmail.com> References: <202102222027.11MKRtcl033616@gitrepo.freebsd.org> <CA%2BZ_v8rKZjuGzbOEbHNJxQ1uU5ZfjpoK6C3SOYNJWO2qxqOwzw@mail.gmail.com> <CACNAnaFWbfS2W-ayW-WnCzD-vJzBH2mYYx95Cgsha5k07hHXRQ@mail.gmail.com> <ef7f77caf1cc07b5963c52a0b720e5d4@freebsd.org> <CACNAnaE4XaAR-S9scg4iG9FOa9b1dHjsuh8ocUpN=NqEip6cKA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2021-02-24 22:02, Kyle Evans wrote: > On Wed, Feb 24, 2021 at 11:53 PM James Gritton <jamie@freebsd.org> > wrote: >> >> On 2021-02-24 21:29, Kyle Evans wrote: >> > On Tue, Feb 23, 2021 at 7:16 AM Alexander Richardson >> > <arichardson@freebsd.org> wrote: >> >> >> >> On Mon, 22 Feb 2021 at 20:28, Jamie Gritton <jamie@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@FreeBSD.org> >> >> > AuthorDate: 2021-02-22 20:27:44 +0000 >> >> > Commit: Jamie Gritton <jamie@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6effcf7f64ec5efec1c275d4c7798017>