Date: Fri, 17 Mar 2017 11:26:43 -0700 From: Bryan Drewery <bdrewery@FreeBSD.org> To: freebsd-current@freebsd.org Subject: Re: r314708: panic: tdsendsignal: ksi on queue Message-ID: <21c8bb48-df8e-f126-e664-e963274f6d48@FreeBSD.org> In-Reply-To: <20170310104429.GH16105@kib.kiev.ua> References: <d510a9da-8293-ba22-a1e6-75b3ea7ffa1d@FreeBSD.org> <20170309144646.GB16105@kib.kiev.ua> <20170309231151.GA49720@stack.nl> <20170310104429.GH16105@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--tmsQDFvpupFPGQB91xloAm82LFMhmjEXv
Content-Type: multipart/mixed; boundary="s8pxjajXRrMXgUNFqBjCDFaEVmSii711M";
protected-headers="v1"
From: Bryan Drewery <bdrewery@FreeBSD.org>
To: freebsd-current@freebsd.org
Message-ID: <21c8bb48-df8e-f126-e664-e963274f6d48@FreeBSD.org>
Subject: Re: r314708: panic: tdsendsignal: ksi on queue
References: <d510a9da-8293-ba22-a1e6-75b3ea7ffa1d@FreeBSD.org>
<20170309144646.GB16105@kib.kiev.ua> <20170309231151.GA49720@stack.nl>
<20170310104429.GH16105@kib.kiev.ua>
In-Reply-To: <20170310104429.GH16105@kib.kiev.ua>
--s8pxjajXRrMXgUNFqBjCDFaEVmSii711M
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
On 3/10/2017 2:44 AM, Konstantin Belousov wrote:
> On Fri, Mar 10, 2017 at 12:11:51AM +0100, Jilles Tjoelker wrote:
>> This patch introduces a subtle correctness bug. A real SIGCHLD ksiginf=
o
>> should always be the zombie's p_ksi; otherwise, the siginfo may be los=
t
>> if there are too many signals pending for the target process or in the=
>> system. If the siginfo is lost and the reaper normally passes si_pid t=
o
>> waitpid() or similar (instead of passing WAIT_ANY or P_ALL), a zombie
>> will remain until the reaper terminates.
>>
>> Conceptually the siginfo is sent to one process at a time only, so the=
>> bug is an artifact of the implementation. Perhaps the piece of code
>> added in r309886 can be moved or the ksiginfo can be removed from the
>> parent's queue.
>>
>> If such a fix is not possible, it may be better to send a bare SIGCHLD=
>> (si_code is SI_KERNEL or 0, depending on how many signals are pending)=
>> in this situation and document that reapers must use WAIT_ANY or P_ALL=
=2E
>> (However, compared to the pre-r309886 situation they can still use
>> SIGCHLD to get notified when to call waitpid() or similar.)
>>
>=20
> IMO it is acceptable for reaper to know and handle the case of lost
> siginfo. But also it seems to be not too hard to guarantee the queueing=
> of the SIGCHLD for zombie re-parerning.
>=20
> diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
> index c524fe5df37..1a1dcc3a4c7 100644
> --- a/sys/kern/kern_exit.c
> +++ b/sys/kern/kern_exit.c
> @@ -189,6 +189,7 @@ exit1(struct thread *td, int rval, int signo)
> {
> struct proc *p, *nq, *q, *t;
> struct thread *tdt;
> + ksiginfo_t *ksi, *ksi1;
> =20
> mtx_assert(&Giant, MA_NOTOWNED);
> KASSERT(rval =3D=3D 0 || signo =3D=3D 0, ("exit1 rv %d sig %d", rval,=
signo));
> @@ -449,14 +450,23 @@ exit1(struct thread *td, int rval, int signo)
> wakeup(q->p_reaper);
> for (; q !=3D NULL; q =3D nq) {
> nq =3D LIST_NEXT(q, p_sibling);
> + ksi =3D ksiginfo_alloc(TRUE);
> PROC_LOCK(q);
> q->p_sigparent =3D SIGCHLD;
> =20
> if (!(q->p_flag & P_TRACED)) {
> proc_reparent(q, q->p_reaper);
> if (q->p_state =3D=3D PRS_ZOMBIE) {
> + if (q->p_ksi =3D=3D NULL) {
> + ksi1 =3D NULL;
> + } else {
> + ksiginfo_copy(q->p_ksi, ksi);
> + ksi->ksi_flags |=3D KSI_INS;
> + ksi1 =3D ksi;
> + ksi =3D NULL;
> + }
> PROC_LOCK(q->p_reaper);
> - pksignal(q->p_reaper, SIGCHLD, q->p_ksi);
> + pksignal(q->p_reaper, SIGCHLD, ksi1);
> PROC_UNLOCK(q->p_reaper);
> }
> } else {
> @@ -489,6 +499,8 @@ exit1(struct thread *td, int rval, int signo)
> kern_psignal(q, SIGKILL);
> }
> PROC_UNLOCK(q);
> + if (ksi !=3D NULL)
> + ksiginfo_free(ksi);
> }
> =20
> /*
Ping? Is this still progressing to be committed?
--=20
Regards,
Bryan Drewery
--s8pxjajXRrMXgUNFqBjCDFaEVmSii711M--
--tmsQDFvpupFPGQB91xloAm82LFMhmjEXv
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQEcBAEBAgAGBQJYzCpmAAoJEDXXcbtuRpfP/5UIAMH80G+OVLVc+UOitpwwVvw2
ZsudM2RenoHTZmGKXCIOEyGVRoGqcAFcL2+bzIXK8tWs6ORTvgfmQXJ1esv9Der+
pLAC/ezJ/gBQGuw863uvJEBj35TXhCzdAa01P++P+/R13v3aOJBBHPOqGNez6swz
HuoE1mXKi44YUidFOWX/vVaxlHQZVgLbGHWWBCBj1cB8XU+m+vAS+iyLmpeD9y88
CX1DZDwFolZBhMJB4qz1Ce7tIqzj+8J13xhAlFl5IjNkF00RLlBXY75cr5Y9yajx
8EDufXLmCsAYdJK44tZGw2SnyVyf2EoTdFQXM1iQPTdwGh5Kk+mA9xFTYx99kCw=
=wJFS
-----END PGP SIGNATURE-----
--tmsQDFvpupFPGQB91xloAm82LFMhmjEXv--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?21c8bb48-df8e-f126-e664-e963274f6d48>