All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ivan Delalande <colona@arista.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: Dmitry Safonov <0x7f454c46@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Andy Lutomirski <luto@kernel.org>
Subject: [PATCH v2] exec: don't force_sigsegv processes with a pending fatal signal
Date: Mon, 4 Feb 2019 18:53:08 -0800	[thread overview]
Message-ID: <20190205025308.GA24455@visor> (raw)

We were seeing unexplained segfaults in coreutils processes and other
basic utilities on systems with print-fatal-signals enabled:

	[  311.001986] potentially unexpected fatal signal 11.
	[  311.001993] CPU: 3 PID: 4565 Comm: tail Tainted: P           O    4.9.100.Ar-8497547.eostrunkkernel49 #1
	[  311.001995] task: ffff88021431b400 task.stack: ffffc90004cec000
	[  311.001997] RIP: 0023:[<00000000f7722c09>]  [<00000000f7722c09>] 0xf7722c09
	[  311.002003] RSP: 002b:00000000ffcc8aa4  EFLAGS: 00000296
	[  311.002004] RAX: fffffffffffffff2 RBX: 0000000057efc530 RCX: 0000000057efdb68
	[  311.002006] RDX: 0000000057effb60 RSI: 0000000057efdb68 RDI: 00000000f768f000
	[  311.002007] RBP: 0000000057efc530 R08: 0000000000000000 R09: 0000000000000000
	[  311.002008] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
	[  311.002009] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
	[  311.002011] FS:  0000000000000000(0000) GS:ffff88021e980000(0000) knlGS:0000000000000000
	[  311.002013] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
	[  311.002014] CR2: 00000000f77bf097 CR3: 0000000150f6f000 CR4: 00000000000406f0

We tracked these crashes down to binfmt_elf failing to load segments
for ld.so inside the kernel. Digging further, the actual problem
seems to occur when a process gets sigkilled while it is still being
loaded by the kernel. In our case when _do_page_fault goes for a retry
it will return early as it first checks for fatal_signal_pending(), so
load_elf_interp also returns with error and as a result
search_binary_handler will force_sigsegv() which is pretty confusing as
nothing actually failed here.


v2: add a message when load_binary fails, add a check for fatal signals
in signal_delivered (avoiding a single check in force_sigsegv as other
architectures use it directly and may have different expectations).

Thanks to Dmitry Safonov and Oleg Nesterov for their comments and
suggestions.

Fixes: 19d860a140be ("handle suicide on late failure exits in execve() in search_binary_handler()")
Reference: https://lkml.org/lkml/2013/2/14/5
Signed-off-by: Ivan Delalande <colona@arista.com>
---
 fs/exec.c       | 7 ++++++-
 kernel/signal.c | 6 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index fb72d36f7823..caf584064f89 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1660,7 +1660,12 @@ int search_binary_handler(struct linux_binprm *bprm)
 		if (retval < 0 && !bprm->mm) {
 			/* we got to flush_old_exec() and failed after it */
 			read_unlock(&binfmt_lock);
-			force_sigsegv(SIGSEGV, current);
+			if (!fatal_signal_pending(current)) {
+				if (print_fatal_signals)
+					pr_info("load_binary() failed: %d\n",
+						retval);
+				force_sigsegv(SIGSEGV, current);
+			}
 			return retval;
 		}
 		if (retval != -ENOEXEC || !bprm->file) {
diff --git a/kernel/signal.c b/kernel/signal.c
index e1d7ad8e6ab1..674076e63624 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2552,10 +2552,10 @@ static void signal_delivered(struct ksignal *ksig, int stepping)
 
 void signal_setup_done(int failed, struct ksignal *ksig, int stepping)
 {
-	if (failed)
-		force_sigsegv(ksig->sig, current);
-	else
+	if (!failed)
 		signal_delivered(ksig, stepping);
+	else if (!fatal_signal_pending(current))
+		force_sigsegv(ksig->sig, current);
 }
 
 /*
-- 
2.20.1

             reply	other threads:[~2019-02-05  3:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-05  2:53 Ivan Delalande [this message]
2019-02-05 21:11 ` [PATCH v2] exec: don't force_sigsegv processes with a pending fatal signal Andrew Morton
2019-02-06  3:10   ` Ivan Delalande
2019-02-08  5:13     ` Eric W. Biederman
2019-02-09  0:16       ` Ivan Delalande
2019-02-10 17:05         ` Eric W. Biederman
2019-02-11 23:25           ` Ivan Delalande
2019-02-11 16:02         ` Oleg Nesterov
2019-02-11 17:12 ` Oleg Nesterov
2019-02-11 23:20   ` Ivan Delalande

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190205025308.GA24455@visor \
    --to=colona@arista.com \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=oleg@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.