linux-kernel - Re: [PATCH 2/8] kill tracehook_notify

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: <20110627082443.GA30101@htj.dyndns.org>
Date:	Mon, 27 Jun 2011 10:24:43 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	hch@...radead.org
Subject: Re: [PATCH 2/8] kill tracehook_notify_death()

Hello, Oleg.

On Sun, Jun 26, 2011 at 10:51:57PM +0200, Oleg Nesterov wrote:
> > Yeap, we've discussed this before and this indeed is odd.  However, is
> > there something ptracer can't do with PTRACE_EVENT_EXIT instead?
> 
> Firstly, I think PTRACE_EVENT_EXIT should not stop the tracee if it
> was SIGKILL'ed. Even if the tracee stops, it can be killed later.
> The tracer can't detach after that, it can't even wait() to detecte
> a zombie leader.

For SIGKILL, yes, it is different, but if PTRACE_EVENT_EXIT is enough
for all other cases, I think we're mostly set.  BTW, it seems like we
would actually stop at PTRACE_EVENT_EXIT even after SIGKILL.  This is
wrong & racy.  may_ptrace_stop() should be checking for
sigkill_pending(), right?

> > Heh, I think this needs to be prettier even at the cost of an inline
> > function.
> 
> May be, but the code is sooooo simple. In fact I thought about the helper,
> but can't find a good name.
> 
> Anyway, this is so minor, unless you strongly object I am going to push
> this patch as is. We can add a helper later although I don't think it is
> needed.
> 
> The same logic could be written as
> 
> 	if (thread_group_empty(tsk)) {
> 		int sig = ptrace_reparented(tsk) ?
> 			SIGCHLD : tsk->exit_signal;
> 		autoreap = do_notify_parent(tsk, sig);
> 	} else if (task->ptrace) {
> 		autoreap = do_notify_parent(tsk, SIGCHLD);
> 	} else {
> 		autoreap = !thread_group_leader();
> 	}
> 
> note that it certainly looks "prettier". However, personaly I strongly
> prefer the non-pretty code above, imho it is more straighforward and
> understandable. It is hardly possible to misread/misunderstand it.

Well, it's cosmetic after all and you're the maintainer.  I don't have
any major problem with the original.  Please go ahead.

Thanks.

-- 
tejun
--
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/