linux-kernel - Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace

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: <aS2wj3j5qfoWDt2p@redhat.com>
Date: Mon, 1 Dec 2025 16:13:19 +0100
From: Oleg Nesterov <oleg@...hat.com>
To: Bernd Edlinger <bernd.edlinger@...mail.de>
Cc: Christian Brauner <brauner@...nel.org>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@...il.com>, Kees Cook <kees@...nel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Will Drewry <wad@...omium.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Hocko <mhocko@...e.com>, Serge Hallyn <serge@...lyn.com>,
	James Morris <jamorris@...ux.microsoft.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Yafang Shao <laoar.shao@...il.com>, Helge Deller <deller@....de>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Adrian Reber <areber@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>, Jens Axboe <axboe@...nel.dk>,
	Alexei Starovoitov <ast@...nel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
	linux-security-module@...r.kernel.org,
	tiozhang <tiozhang@...iglobal.com>,
	Luis Chamberlain <mcgrof@...nel.org>,
	"Paulo Alcantara (SUSE)" <pc@...guebit.com>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Frederic Weisbecker <frederic@...nel.org>,
	YueHaibing <yuehaibing@...wei.com>,
	Paul Moore <paul@...l-moore.com>, Aleksa Sarai <cyphar@...har.com>,
	Stefan Roesch <shr@...kernel.io>, Chao Yu <chao@...nel.org>,
	xu xin <xu.xin16@....com.cn>, Jeff Layton <jlayton@...nel.org>,
	Jan Kara <jack@...e.cz>, David Hildenbrand <david@...hat.com>,
	Dave Chinner <dchinner@...hat.com>, Shuah Khan <shuah@...nel.org>,
	Elena Reshetova <elena.reshetova@...el.com>,
	David Windsor <dwindsor@...il.com>,
	Mateusz Guzik <mjguzik@...il.com>, Ard Biesheuvel <ardb@...nel.org>,
	"Joel Fernandes (Google)" <joel@...lfernandes.org>,
	"Matthew Wilcox (Oracle)" <willy@...radead.org>,
	Hans Liljestrand <ishkamiel@...il.com>,
	Penglei Jiang <superman.xpt@...il.com>,
	Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
	Adrian Ratiu <adrian.ratiu@...labora.com>,
	Ingo Molnar <mingo@...nel.org>,
	"Peter Zijlstra (Intel)" <peterz@...radead.org>,
	Cyrill Gorcunov <gorcunov@...il.com>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH v17] exec: Fix dead-lock in de_thread with ptrace_attach

On 11/29, Bernd Edlinger wrote:
>
> On 11/23/25 19:32, Oleg Nesterov wrote:
> > I don't follow. Do you mean PREEMPT_RT ?
> >
> > If yes. In this case spin_lock_irq() is rt_spin_lock() which doesn't disable irqs,
> > it does rt_lock_lock() (takes rt_mutex) + migrate_disable().
> >
> > I do think that spin/mutex/whatever_unlock() is always safe. In any order, and
> > regardless of RT.
> >
>
> It is hard to follow how linux implements that spin_lock_irq exactly,

Yes ;)

> but
> to me it looks like it is done this way:
>
> include/linux/spinlock_api_smp.h:static inline void __raw_spin_lock_irq(raw_spinlock_t *lock)
> include/linux/spinlock_api_smp.h-{
> include/linux/spinlock_api_smp.h-       local_irq_disable();
> include/linux/spinlock_api_smp.h-       preempt_disable();
> include/linux/spinlock_api_smp.h-       spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
> include/linux/spinlock_api_smp.h-       LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
> include/linux/spinlock_api_smp.h-}

Again, I will assume you mean RT.

In this case spinlock_t and raw_spinlock_t are not the same thing.

include/linux/spinlock_types.h:

	typedef struct spinlock {
		struct rt_mutex_base	lock;
	#ifdef CONFIG_DEBUG_LOCK_ALLOC
		struct lockdep_map	dep_map;
	#endif
	} spinlock_t;

include/linux/spinlock_rt.h:

	static __always_inline void spin_lock_irq(spinlock_t *lock)
	{
		rt_spin_lock(lock);
	}

rt_spin_lock() doesn't disable irqs, it takes "rt_mutex_base lock" and
disables migration.


> so an explicit task switch while locka_irq_disable looks
> very dangerous to me.

raw_spin_lock_irq() disables irqs/preemption regardless of RT, task switch
is not possible.

> Do you know other places where such
> a code pattern is used?

For example, double_lock_irq(). See task_numa_group(),

	double_lock_irq(&my_grp->lock, &grp->lock);

	....

	spin_unlock(&my_grp->lock);
	spin_unlock_irq(&grp->lock);

this can unlock the locks in reverse order.

I am sure there are more examples.

> I do just ask, because a close look at those might reveal
> some serious bugs, WDYT?

See above, I don't understand your concerns...

Oleg.