All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: syzbot <syzbot+3485e3773f7da290eecc@syzkaller.appspotmail.com>,
	axboe@kernel.dk, christian@brauner.io,
	linux-kernel@vger.kernel.org, liuzhiqiang26@huawei.com,
	syzkaller-bugs@googlegroups.com, Tejun Heo <tj@kernel.org>
Subject: Re: WARNING in get_signal
Date: Tue, 6 Oct 2020 19:05:23 +0200	[thread overview]
Message-ID: <20201006170523.GC9995@redhat.com> (raw)
In-Reply-To: <20201005163016.GB9995@redhat.com>

On 10/05, Oleg Nesterov wrote:
>
> On 10/05, Oleg Nesterov wrote:
> >
> > > It looks like this code was introduced in commit 73ddff2bee15 ("job
> > > control: introduce JOBCTL_TRAP_STOP and use it for group stop trap").
> >
> > Yes, but I bet this was broken later, _may be_ by 924de3b8c9410c4.
>
> No, it seems this bug is really old. I'll try to make the fix tomorrow.

I still do not see a good fix. I am crying ;)

For the moment, lets forget about this problem. 924de3b8c9410c4 was wrong
anyway, task_join_group_stop() should be fixed:

	- if current is traced, "jobctl & JOBCTL_STOP_PENDING" is not
	  enough, we need to check SIGNAL_STOP_STOPPED/group_stop_count

	- if the new thread is traced, task_join_group_stop() should do
	  nothing, we should rely on ptrace_init_task()


Now lets return to this bug report. This (incomplete) test-case

	void *tf(void *arg)
	{
		return NULL;
	}

	int main(void)
	{
		int pid = fork();
		if (!pid) {
			setpgrp();
			kill(getpid(), SIGTSTP);

			pthread_t th;
			pthread_create(&th, NULL, tf, NULL);

			return 0;
		}

		waitpid(pid, NULL, WSTOPPED);

		ptrace(PTRACE_SEIZE, pid, 0, PTRACE_O_TRACECLONE);
		waitpid(pid, NULL, 0);

		ptrace(PTRACE_CONT, pid, 0,0);
		waitpid(pid, NULL, 0);

		int status;
		int thr = waitpid(-1, &status, 0);
		printf("pids: %d %d status: %x\n", pid, thr, status);

		return 0;
	}

triggers WARN_ON_ONCE(!signr) in do_jobctl_trap() and shows that the
auto-attached sub-thread reports the wrong status.

This patch

	--- x/include/linux/ptrace.h
	+++ x/include/linux/ptrace.h
	@@ -218,7 +218,7 @@ static inline void ptrace_init_task(stru
			__ptrace_link(child, current->parent, current->ptracer_cred);
	 
			if (child->ptrace & PT_SEIZED)
	-			task_set_jobctl_pending(child, JOBCTL_TRAP_STOP);
	+			task_set_jobctl_pending(child, JOBCTL_TRAP_STOP|SIGTRAP);
			else
				sigaddset(&child->pending.signal, SIGSTOP);
		}

should fix the problem, but it is not enough even if we forget about
task_join_group_stop().

	- it is not clear to me if the new thread should join the group stop
	  after (say) PTRACE_CONT. If yes, it is not clear how can we do this.

	- in any case it should stop after ptrace_detach(), but in this case
	  jobctl & JOBCTL_STOP_SIGMASK == SIGTRAP doesn't look right.

	  So perhaps we can change the patch above to use
	  current->jobctl & JOBCTL_STOP_SIGMASK instead of SIGTRAP ?

	  This too doesn't look good, the 1st ptrace_stop() should probably
	  always report SIGTRAP...

Oleg.


  reply	other threads:[~2020-10-06 17:05 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-02 15:48 WARNING in get_signal syzbot
2020-10-02 15:56 ` Eric W. Biederman
2020-10-05 13:49   ` Oleg Nesterov
2020-10-05 16:30     ` Oleg Nesterov
2020-10-06 17:05       ` Oleg Nesterov [this message]
2020-10-15 12:49         ` Oleg Nesterov
2020-10-19 13:42 ` [PATCH] ptrace: fix task_join_group_stop() for the case when current is traced Oleg Nesterov

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=20201006170523.GC9995@redhat.com \
    --to=oleg@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=christian@brauner.io \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuzhiqiang26@huawei.com \
    --cc=syzbot+3485e3773f7da290eecc@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tj@kernel.org \
    /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.