2022-04-14 15:06:14

by Duoming Zhou

[permalink] [raw]
Subject: [PATCH 3/3] drivers: nfc: nfcmrvl: fix use-after-free bug in nfcmrvl_fw_dnld_start()

There are potential use-after-free bug in nfcmrvl_fw_dnld_start().
The race between nfcmrvl_disconnect() and nfcmrvl_fw_dnld_start()
can be shown as below:

(USE) | (FREE)
| nfcmrvl_disconnect
| nfcmrvl_nci_unregister_dev
| nfcmrvl_fw_dnld_abort
| fw_dnld_over
nfcmrvl_fw_dnld_start | release_firmware
... | kfree(fw) //(1)
priv->fw_dnld.fw->data //(2)| ...
... |

The firmware is deallocate in position (1), but it is used in position
(2), which leads to UAF bug.

This patch add spin_lock() and check in nfcmrvl_fw_dnld_start()
in order to synchronize with other threads that could free firmware.
Therefore, the UAF bug could be prevented.

Fixes: 3194c6870158e3 ("NFC: nfcmrvl: add firmware download support")
Signed-off-by: Duoming Zhou <[email protected]>
---
drivers/nfc/nfcmrvl/fw_dnld.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/nfcmrvl/fw_dnld.c b/drivers/nfc/nfcmrvl/fw_dnld.c
index bb9e7e2bdec..910f6eaec65 100644
--- a/drivers/nfc/nfcmrvl/fw_dnld.c
+++ b/drivers/nfc/nfcmrvl/fw_dnld.c
@@ -511,7 +511,10 @@ int nfcmrvl_fw_dnld_start(struct nci_dev *ndev, const char *firmware_name)
return -ENOENT;
}

- fw_dnld->header = (const struct nfcmrvl_fw *) priv->fw_dnld.fw->data;
+ spin_lock(&priv->fw_dnld.lock);
+ if (priv->fw_dnld.fw)
+ fw_dnld->header = (const struct nfcmrvl_fw *)priv->fw_dnld.fw->data;
+ spin_unlock(&priv->fw_dnld.lock);

if (fw_dnld->header->magic != NFCMRVL_FW_MAGIC ||
fw_dnld->header->phy != priv->phy) {
--
2.17.1