The firmware download work function holds `info_lock' while calling the
nxp_nci_fw_send(), which waits for a completion.

The completion is signalled by the IRQ thread handler, which also takes
`info_lock', causing a deadlock.

Fix by dropping the mutex while waiting for the completion.

Fixes: dece45855a8b ("NFC: nxp-nci: Add support for NXP NCI chips")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
Changes since v1:
- Fixes tag
- Cc for stable
---
 drivers/nfc/nxp-nci/firmware.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nfc/nxp-nci/firmware.c b/drivers/nfc/nxp-nci/firmware.c
index a9533977aff8..38fcac26ce56 100644
--- a/drivers/nfc/nxp-nci/firmware.c
+++ b/drivers/nfc/nxp-nci/firmware.c
@@ -154,8 +154,13 @@ static int nxp_nci_fw_send(struct nxp_nci_info *info)
 		if (fw_info->fw)
 			schedule_work(&fw_info->work);
 	} else {
+		mutex_unlock(&info->info_lock);
+
 		completion_rc = wait_for_completion_interruptible_timeout(
 			&fw_info->cmd_completion, NXP_NCI_FW_ANSWER_TIMEOUT);
+
+		mutex_lock(&info->info_lock);
+
 		if (completion_rc == 0)
 			return -ETIMEDOUT;
 	}
-- 
2.47.3

On 10/08/2026 10:21, Ian Ray wrote:
> The firmware download work function holds `info_lock' while calling the
> nxp_nci_fw_send(), which waits for a completion.
> 
> The completion is signalled by the IRQ thread handler, which also takes
> `info_lock', causing a deadlock.
> 
> Fix by dropping the mutex while waiting for the completion.
> 
> Fixes: dece45855a8b ("NFC: nxp-nci: Add support for NXP NCI chips")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
> ---
> Changes since v1:
> - Fixes tag
> - Cc for stable
> ---
>   drivers/nfc/nxp-nci/firmware.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 

Hello Ian,

I checked the Sashiko review and it seems found an issue (feel free to ignore 
the 2nd pre-existing), could you check please (below)?

Thanks
David

--- Patch [1]: [PATCH v2] NFC: nxp-nci: fix deadlock between firmware download 
and IRQ handler ---
commit aeb1accbd1a732a455bf37b530649e8d9f406914
Author: Ian Ray <ian.ray@gehealthcare.com>
Subject: NFC: nxp-nci: fix deadlock between firmware download and IRQ handler

This commit addresses a deadlock during firmware download by dropping the
info_lock mutex while waiting for the IRQ handler to signal completion.

 > diff --git a/drivers/nfc/nxp-nci/firmware.c b/drivers/nfc/nxp-nci/firmware.c
 > index a9533977aff8e..38fcac26ce563 100644
 > --- a/drivers/nfc/nxp-nci/firmware.c
 > +++ b/drivers/nfc/nxp-nci/firmware.c
 > @@ -154,8 +154,13 @@ static int nxp_nci_fw_send(struct nxp_nci_info *info)
 >  		if (fw_info->fw)
 >  			schedule_work(&fw_info->work);
 >  	} else {
 > +		mutex_unlock(&info->info_lock);
 > +
 >  		completion_rc = wait_for_completion_interruptible_timeout(
 >  			&fw_info->cmd_completion, NXP_NCI_FW_ANSWER_TIMEOUT);
 > +
 > +		mutex_lock(&info->info_lock);
 > +

If wait_for_completion_interruptible_timeout() times out, could this open a
race condition leading to a use-after-free?

When the timeout expires, the IRQ handler might run before info_lock is
re-acquired here. Since the timeout path hasn't cleaned up fw_info->fw yet,
the IRQ handler would see fw_info->fw != NULL and erroneously call
schedule_work(&fw_info->work) for a late response:

drivers/nfc/nxp-nci/firmware.c:nxp_nci_fw_recv_frame() {
     ...
     if (fw_info->fw)
         schedule_work(&fw_info->work);
     ...
}

After re-acquiring the lock, this function returns -ETIMEDOUT, and the caller
nxp_nci_fw_work_complete() will free the firmware buffer.

Could the spuriously scheduled work then execute and dereference the freed
memory when it calls nxp_nci_fw_send_chunk():

     skb_put_data(skb, fw_info->data + fw_info->written, chunk_len);

 >  		if (completion_rc == 0)
 >  			return -ETIMEDOUT;
 >  	}

This is a pre-existing issue, but does nxp_nci_remove() have a use-after-free
if a firmware download is currently running?

nxp_nci_remove() calls nxp_nci_fw_work_complete() to free the firmware buffer
before cancelling the work queue:

drivers/nfc/nxp-nci/core.c:nxp_nci_remove() {
     ...
     if (info->mode == NXP_NCI_MODE_FW)
         nxp_nci_fw_work_complete(info, -ESHUTDOWN);
     cancel_work_sync(&info->fw_info.work);
     ...
}

Since nxp_nci_fw_work_complete() frees fw_info->fw without holding info_lock,
could the concurrent nxp_nci_fw_work() thread be actively copying data from
fw_info->data, resulting in a use-after-free or a double free if the worker
finishes and calls nxp_nci_fw_work_complete() as well?

This is a pre-existing issue, but is there a potential integer underflow and
out-of-bounds read earlier in nxp_nci_fw_send()?

Before dropping the lock, the firmware size is modified without a length
check:

drivers/nfc/nxp-nci/firmware.c:nxp_nci_fw_send() {
     ...
     if (fw_info->written == 0) {
         fw_info->frame_size = get_unaligned_be16(fw_info->data) &
                               NXP_NCI_FW_FRAME_LEN_MASK;
         fw_info->data += NXP_NCI_FW_HDR_LEN;
         fw_info->size -= NXP_NCI_FW_HDR_LEN;
     }
     ...
}

If the provided firmware file is smaller than NXP_NCI_FW_HDR_LEN (2 bytes),
fw_info->size (which is unsigned) will underflow to a very large value.

Would this cause the subsequent safety check to be bypassed:

     if (fw_info->frame_size > fw_info->size)
         return -EMSGSIZE;

and allow nxp_nci_fw_send_chunk() to read out-of-bounds heap memory?