Re: [PATCH nfc] nfc: nci: fix double completion race in nci_data_exchange_complete

On 26/05/2026 12:31, Zhenghang Xiao wrote:
> nci_close_device() and nci_rx_work can both call
> nci_data_exchange_complete() concurrently.  After commit 4527025d440ce8
> ("nfc: nci: fix circular locking dependency in nci_close_device") moved
> flush_workqueue(ndev->rx_wq) after mutex_unlock(&ndev->req_lock),
> rx_work is no longer serialized with the explicit completion call in the
> close path.  Both callers read the non-NULL callback pointer and invoke
> rawsock_data_exchange_complete(), which calls sock_put() -- but only one
> sock_hold() was taken, so the second sock_put() underflows the refcount
> and frees the socket while it is still in use.
> 
> Replace the bare clear_bit(NCI_DATA_EXCHANGE) with
> test_and_clear_bit() so that only the first caller proceeds to invoke
> the callback.
> 
> Fixes: 4527025d440c ("nfc: nci: fix circular locking dependency in nci_close_device")
> Signed-off-by: Zhenghang Xiao <[email protected]>
> ---
>   net/nfc/nci/data.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/net/nfc/nci/data.c b/net/nfc/nci/data.c
> index 5f98c73db5af..4253edea5c8d 100644
> --- a/net/nfc/nci/data.c
> +++ b/net/nfc/nci/data.c
> @@ -46,11 +46,11 @@ void nci_data_exchange_complete(struct nci_dev *ndev, struct sk_buff *skb,
>   	timer_delete_sync(&ndev->data_timer);
>   	clear_bit(NCI_DATA_EXCHANGE_TO, &ndev->flags);
>   
> -	/* Mark the exchange as done before calling the callback.
> -	 * The callback (e.g. rawsock_data_exchange_complete) may
> -	 * want to immediately queue another data exchange.
> -	 */
> -	clear_bit(NCI_DATA_EXCHANGE, &ndev->flags);
> +	/* Claim completion atomically -- both close and rx_work may race here */
> +	if (!test_and_clear_bit(NCI_DATA_EXCHANGE, &ndev->flags)) {
> +		kfree_skb(skb);
> +		return;
> +	}
>   
>   	if (cb) {
>   		/* forward skb to nfc core */

Reviewed-by: David Heidelberg <[email protected]>

-- 
David Heidelberg