netdev - Re: [PATCH net 3/5] rxrpc: Fix notification vs call-release vs recvmsg

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250716190001.GR721198@horms.kernel.org>
Date: Wed, 16 Jul 2025 20:00:01 +0100
From: Simon Horman <horms@...nel.org>
To: David Howells <dhowells@...hat.com>
Cc: netdev@...r.kernel.org, Marc Dionne <marc.dionne@...istor.com>,
	Jakub Kicinski <kuba@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
	linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Jeffrey Altman <jaltman@...istor.com>,
	"Junvyyang, Tencent Zhuque Lab" <zhuque@...cent.com>,
	LePremierHomme <kwqcheii@...ton.me>, stable@...r.kernel.org
Subject: Re: [PATCH net 3/5] rxrpc: Fix notification vs call-release vs
 recvmsg

On Wed, Jul 16, 2025 at 12:53:02PM +0100, David Howells wrote:
> When a call is released, rxrpc takes the spinlock and removes it from
> ->recvmsg_q in an effort to prevent racing recvmsg() invocations from
> seeing the same call.  Now, rxrpc_recvmsg() only takes the spinlock when
> actually removing a call from the queue; it doesn't, however, take it in
> the lead up to that when it checks to see if the queue is empty.  It *does*
> hold the socket lock, which prevents a recvmsg/recvmsg race - but this
> doesn't prevent sendmsg from ending the call because sendmsg() drops the
> socket lock and relies on the call->user_mutex.
> 
> Fix this by firstly removing the bit in rxrpc_release_call() that dequeues
> the released call and, instead, rely on recvmsg() to simply discard
> released calls (done in a preceding fix).
> 
> Secondly, rxrpc_notify_socket() is abandoned if the call is already marked
> as released rather than trying to be clever by setting both pointers in
> call->recvmsg_link to NULL to trick list_empty().  This isn't perfect and
> can still race, resulting in a released call on the queue, but recvmsg()
> will now clean that up.
> 
> Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by userspace and kernel both")
> Signed-off-by: David Howells <dhowells@...hat.com>
> Reviewed-by: Jeffrey Altman <jaltman@...istor.com>

...

> diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c

...

> @@ -638,6 +628,12 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
>  		rxrpc_put_call(call, rxrpc_call_put_release_sock);
>  	}
>  
> +	while ((call = list_first_entry_or_null(&rx->recvmsg_q,
> +						struct rxrpc_call, recvmsg_link))) {
> +		list_del_init(&call->recvmsg_link);
> +		rxrpc_put_call(call, rxrpc_call_put_release_recvmsg_q);
> +	}
> +
>  	_leave("");
>  }
>  

Hi David,

I believe it is addressed in patch 5/5.
But unfortunately this change breaks bisection.

  .../call_object.c:634:24: error: use of undeclared identifier 'rxrpc_call_put_release_recvmsg_q'
    634 |                 rxrpc_put_call(call, rxrpc_call_put_release_recvmsg_q);
        |                                      ^

-- 
pw-bot: changes-requested