linux-kernel - Re: [PATCH 0/3] coredump: fix the ancient signal problems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFx-9Ajb13JfxghNMzoHMJWDtH=a0DZfz2Q-eCBnHyVDPQ@mail.gmail.com>
Date:	Sun, 17 Feb 2013 11:34:10 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Ingo Molnar <mingo@...hat.com>,
	Mandeep Singh Baines <msb@...omium.org>,
	Neil Horman <nhorman@...hat.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Roland McGrath <roland@...k.frob.com>,
	Tejun Heo <tj@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] coredump: fix the ancient signal problems

On Sun, Feb 17, 2013 at 11:18 AM, Oleg Nesterov <oleg@...hat.com> wrote:
>
> Linus, et al, could you please ack/nack the intent? Of course I will
> appreciate if you can review the code, but what I am actually worried
> about is the user-visible change: the coredumping becomes killable but
> only by the _explicit_ SIGKILL, other fatal signals are "ignored".

That isn't a problem. In fact, we already have logic that makes the
act of writing a file be killable by SIGKILL (because you really
really want that for network filesystems, for example), so I suspect
that core-dumping was interruptible by SIGKILL even before you made it
explicitly so - simply because the IO itself was.

And even if it wasn't (because maybe the SIGKILL logic doesn't get
triggered due to all the special-case core-dumping code in signal
handling), SIGKILL really is very very special. Having it kill a
coredump in progress sounds fine to me. SIGKILL is used by
users/admins as a last-case thing to get rid of processes that are
problematic for the system, and we should really try to honor it over
just about anything else for that reason. It's the user saying "get
this thing away from me", we should honor it.

That said, I'm not convinced about your particular split of patches.
The first patch introduces that new SIGNAL_GROUP_COREDUMP, and then
the second patch modifies one of the new use cases:

-               tsk->signal->flags |= SIGNAL_GROUP_COREDUMP;
+               tsk->signal->flags = SIGNAL_GROUP_COREDUMP;

and that just smells to me like you tried too hard to split things
into two patches.

And obviously, it will need more testing and review. I wonder if Al
Viro hould be on the cc. He doesn't always get involved, but when it
comes to core-dumping and signal handling, he *sometimes* does, and so
just checking if he has any comments might be worth it.

                         Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/