From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755443AbdDEQUt (ORCPT ); Wed, 5 Apr 2017 12:20:49 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46348 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754876AbdDEQTp (ORCPT ); Wed, 5 Apr 2017 12:19:45 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5067FC04BD44 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=oleg@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5067FC04BD44 Date: Wed, 5 Apr 2017 18:19:32 +0200 From: Oleg Nesterov To: "Eric W. Biederman" Cc: Andrew Morton , Aleksa Sarai , Andy Lutomirski , Attila Fazekas , Jann Horn , Kees Cook , Michal Hocko , Ulrich Obergfell , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org Subject: Re: [RFC][PATCH v2 1/5] ptrace: Don't wait in PTRACE_O_TRACEEXIT for exec or coredump Message-ID: <20170405161932.GE14536@redhat.com> References: <20170303173326.GA17899@redhat.com> <87tw7axlr0.fsf@xmission.com> <87d1dyw5iw.fsf@xmission.com> <87tw7aunuh.fsf@xmission.com> <87lgsmunmj.fsf_-_@xmission.com> <20170304170312.GB13131@redhat.com> <8760ir192p.fsf@xmission.com> <878tnkpv8h.fsf_-_@xmission.com> <874ly6a0h1.fsf_-_@xmission.com> <87wpb28luj.fsf_-_@xmission.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87wpb28luj.fsf_-_@xmission.com> User-Agent: Mutt/1.5.18 (2008-05-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 05 Apr 2017 16:19:45 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 04/02, Eric W. Biederman wrote: > > In the case of exec and coredump which have many interesting deadlock > opportunities So this patch is very close to my 2/2 one-liner, except - you removed the current->mm == current->parent->mm check I didn't do this on purpose, because even the->core_state is not really needed if we check ->group_exit_task, this need more changes anyway, but I won't argue. - With your patch we send the notification to debugger even if we are not going to stop. This is not wrong, but why? This is pointless, nobody rely on SIGCHLD, if nothing else it doesn't queue. Again, I won't argue, but this complicates both the patch and the code for no reason. Unless I missed something. > Keep sending the signal to the tracer so that this appears like > the worst case where someone else sent the process a SIGKILL before > the tracer could react. So all non-buggy tracers must support > this case. Well, I can't understand the changelog. Sure, debugger must support this case, but obviously this can break things anyway. For example. The coredumping thread must stop in PTRACE_EVENT_EXIT. There is a tool (I don't remember its name) which does ptrace_attach(PTRACE_SEIZE, PTRACE_O_TRACEEXIT) after the coredump was already started, closes the pipe, and reads the registers when this thread actually exits. This patch or my 2/2 should not break it, ->group_exit_task will be cleared after do_coredump(), but unfortunately something else can be broken. So I think the changelog should mention that yes, this is the user visible change which _can_ break something anyway. In short. I will be really happy if this patch comes from you, not me ;) Oleg.