From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 89FDA18B06; Mon, 15 Jan 2024 19:39:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="GknGwOR3" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=VZxWDVpCvSyMMfu91KFYLQxRYf92goXw309/GJka1jA=; b=GknGwOR3WJOANvxNIoJBsht0dF XYsbU6l237FY/y32zn2rCALaUsCZlCV/doukPROyDgBih0N5A4poaUezz78fJAKRRw4spk15ec5Vb wt2fW49Tf5hUy1Hhnn443PL5X7S9d7/gXo4XQwx3EUazTRQxz+Shm5v9T9MI7Iht32IIsAs0U1+5m qZsX/gGVMtWr+31/fvBSc0u6UiYa6uL+Zu+yPn0SAL9/S+oSphBGk8TFTU9cdTmhlCtPPQICjWSBx bQaKlCH6vSEgmCAmZ/tYp4bqLoAYu/ZJOK5RB/ww1iUiq2qK0Pp9RvifgWHu88AeS/d5XVWIE4CNl YlL/2RBA==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1rPSmJ-00Ad0l-L5; Mon, 15 Jan 2024 19:37:47 +0000 Date: Mon, 15 Jan 2024 19:37:47 +0000 From: Matthew Wilcox To: Bernd Edlinger Cc: Alexander Viro , Alexey Dobriyan , Oleg Nesterov , Kees Cook , Andy Lutomirski , Will Drewry , Christian Brauner , Andrew Morton , Michal Hocko , Serge Hallyn , James Morris , Randy Dunlap , Suren Baghdasaryan , Yafang Shao , Helge Deller , "Eric W. Biederman" , Adrian Reber , Thomas Gleixner , Jens Axboe , Alexei Starovoitov , "linux-fsdevel@vger.kernel.org" , "linux-kernel@vger.kernel.org" , linux-kselftest@vger.kernel.org, linux-mm@kvack.org, tiozhang , Luis Chamberlain , "Paulo Alcantara (SUSE)" , Sergey Senozhatsky , Frederic Weisbecker , YueHaibing , Paul Moore , Aleksa Sarai , Stefan Roesch , Chao Yu , xu xin , Jeff Layton , Jan Kara , David Hildenbrand , Dave Chinner , Shuah Khan , Zheng Yejian , Elena Reshetova , David Windsor , Mateusz Guzik , Ard Biesheuvel , "Joel Fernandes (Google)" , Hans Liljestrand Subject: Re: [PATCH v14] exec: Fix dead-lock in de_thread with ptrace_attach Message-ID: References: Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Jan 15, 2024 at 08:22:19PM +0100, Bernd Edlinger wrote: > This introduces signal->exec_bprm, which is used to > fix the case when at least one of the sibling threads > is traced, and therefore the trace process may dead-lock > in ptrace_attach, but de_thread will need to wait for the > tracer to continue execution. Not entirely sure why I've been added to the cc; this doesn't seem like it's even remotely within my realm of expertise. > +++ b/include/linux/cred.h > @@ -153,6 +153,7 @@ extern const struct cred *get_task_cred(struct task_struct *); > extern struct cred *cred_alloc_blank(void); > extern struct cred *prepare_creds(void); > extern struct cred *prepare_exec_creds(void); > +extern bool is_dumpability_changed(const struct cred *, const struct cred *); Using 'extern' for function declarations is deprecated. More importantly, you have two arguments of the same type, and how do I know which one is which if you don't name them? > +++ b/kernel/cred.c > @@ -375,6 +375,28 @@ static bool cred_cap_issubset(const struct cred *set, const struct cred *subset) > return false; > } > > +/** > + * is_dumpability_changed - Will changing creds from old to new > + * affect the dumpability in commit_creds? > + * > + * Return: false - dumpability will not be changed in commit_creds. > + * true - dumpability will be changed to non-dumpable. > + * > + * @old: The old credentials > + * @new: The new credentials > + */ Does kernel-doc really parse this correctly? Normal style would be: /** * is_dumpability_changed - Will changing creds affect dumpability? * @old: The old credentials. * @new: The new credentials. * * If the @new credentials have no elevated privileges compared to the * @old credentials, the task may remain dumpable. Otherwise we have * to mark the task as undumpable to avoid information leaks from higher * to lower privilege domains. * * Return: True if the task will become undumpable. */ > @@ -508,6 +531,14 @@ static int ptrace_traceme(void) > { > int ret = -EPERM; > > + if (mutex_lock_interruptible(¤t->signal->cred_guard_mutex)) > + return -ERESTARTNOINTR; Do you really want this to be interruptible by a timer signal or a window resize event?