linux-kernel - Re: [PATCH RFC 2/4] net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer

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: <20250425011448.86924-1-kuniyu@amazon.com>
Date: Thu, 24 Apr 2025 18:08:46 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <brauner@...nel.org>
CC: <alexander@...alicyn.com>, <bluca@...ian.org>, <daan.j.demeyer@...il.com>,
	<davem@...emloft.net>, <david@...dahead.eu>, <edumazet@...gle.com>,
	<horms@...nel.org>, <jack@...e.cz>, <kuba@...nel.org>, <kuniyu@...zon.com>,
	<lennart@...ttering.net>, <linux-fsdevel@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <me@...dnzj.com>, <netdev@...r.kernel.org>,
	<oleg@...hat.com>, <pabeni@...hat.com>
Subject: Re: [PATCH RFC 2/4] net, pidfs: prepare for handing out pidfds for reaped sk->sk_peer_pid

From: Christian Brauner <brauner@...nel.org>
Date: Thu, 24 Apr 2025 17:19:28 +0200
> > > @@ -643,6 +644,14 @@ static void unix_sock_destructor(struct sock *sk)
> > >  		return;
> > >  	}
> > > 
> > > +	if (sock_flag(sk, SOCK_RCU_FREE)) {
> > > +		pr_info("Attempting to release RCU protected socket with sleeping 
> > > locks: %p\n", sk);
> > > +		return;
> > > +	}
> > 
> > unix-sockets do not use `SOCK_RCU_FREE`,

Right, and I think we won't flag SOCK_RCU_FREE in the future.


> but even if they did, doesn't
> > this flag imply that the destructor is delayed via `call_rcu`, and
> > thus *IS* allowed to sleep? And then, sleeping in the destructor is
> > always safe, isn't it? `SOCK_RCU_FREE` just guarantees that it is
> > delayed for at least an RCU grace period, right? Not sure, what you
> > are getting at here, but I might be missing something obvious as well.
> 
> Callbacks run from call_rcu() can be called from softirq context and in
> general are not allowed to block. That's what queue_rcu_work() is for
> which uses system_unbound_wq.
> 
> > 
> > Regardless, wouldn't you want WARN_ON_ONCE() rather than pr_info?
> 
> Sure.

I prefer DEBUG_NET_WARN_ON_ONCE() or removing it as rcu_sleep_check()
in __might_sleep() has better checks.

The netdev CI enables debug.config, which has CONFIG_DEBUG_ATOMIC_SLEEP
and enables the checks, so adding a test case in
tools/testing/selftests/net/af_unix/scm_pidfd.c will catch the future
regression.