linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivan Delalande <colona@arista.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	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: Re: [PATCH v2] exec: don't force_sigsegv processes with a pending fatal signal
Date: Mon, 11 Feb 2019 15:25:29 -0800	[thread overview]
Message-ID: <20190211232529.GA28428@visor> (raw)
In-Reply-To: <87ftsvmv4f.fsf@xmission.com>

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Sun, Feb 10, 2019 at 11:05:52AM -0600, Eric W. Biederman wrote:
> Ivan Delalande <colona@arista.com> writes:
> > A difference I've noticed with your tree (unrelated to my issue here but
> > that you may want to look at) is when I run my reproducer under
> > strace -f, I'm now getting quite a lot of "Exit of unknown pid 12345
> > ignored" warnings from strace, which I've never seen with mainline.
> > My reproducer simply fork-exec tail processes in a loop, and tries to
> > sigkill them in the parent with a variable delay.
> 
> What was your base tree?

It was just off v5.0-rc5, and I didn't see these warnings on the last
few RCs either. Now I'm seeing them on vanilla v5.0-rc6 as well.

> My best guess is that your SIGKILL is getting there before strace
> realizes the process has been forked.  If we can understand the race
> it is probably worth fixing.
> 
> Any chance you can post your reproducer.

Sure, see the attachment. I think this is the simplest version where
these warnings show up. This one just forks/exec `tail -a` to make it
fail and exit 1 as soon as possible, and progressively increase the
delay between the fork and sigkill to try to hit our original issue,
stopping and restarting only after 10 completions of the child as the
timing varies a fair bit.

Running this program under `strace -f -o /dev/null` prints the warnings
almost instantly on my system.

> It is possible it is my most recent fixes, or it is possible something
> changed from the tree you were testing and the tree you are working
> on.

Thanks,

-- 
Ivan Delalande
Arista Networks

[-- Attachment #2: forksigkilltest.c --]
[-- Type: text/plain, Size: 1219 bytes --]

#define _GNU_SOURCE
#include <time.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <unistd.h>
#include <signal.h>
#include <stdio.h>

int main(void)
{
	pid_t pid;
	int status;
	size_t i, count;
	unsigned long max = 300000, first;
	struct timespec ts = { .tv_nsec = 1 };
	char* const argv[] = {"/bin/tail", "-a", NULL};

	for (i = 0; i < 42000; ++i) {
		for (count = first = 0, ts.tv_nsec = 1;
		     ts.tv_nsec < max && count < 10;
		     ts.tv_nsec += 1) {
			if ((pid = fork())) {
				if (pid < 0)
					continue;
				nanosleep(&ts, NULL);
				kill(pid, SIGKILL);
				if (waitpid(pid, &status, 0) != pid)
					continue;
				if (WIFSIGNALED(status) &&
				    WTERMSIG(status) == 9) {
					continue;
				} else if (WIFEXITED(status) &&
					   WEXITSTATUS(status) == 1) {
					count++;
					if (!first)
						first = ts.tv_nsec;
				} else
					printf("%lu: %x\n", ts.tv_nsec, status);
			} else {
				close(STDOUT_FILENO);
				close(STDERR_FILENO);
				execve("/bin/tail", argv, NULL);
				_exit(2);
			}
		}
		if (max < ts.tv_nsec)
			max = ts.tv_nsec;
		if (count < 10)
			max += 5000;
		printf("break at %lu (max: %lu) count %lu (first at %lu)\n",
		       ts.tv_nsec, max, count, first);
	}

	return 0;
}

  reply	other threads:[~2019-02-11 23:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-05  2:53 [PATCH v2] exec: don't force_sigsegv processes with a pending fatal signal Ivan Delalande
2019-02-05 21:11 ` 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 [this message]
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=20190211232529.GA28428@visor \
    --to=colona@arista.com \
    --cc=0x7f454c46@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).