iv/linux
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

NFC: reorder the logic in nfc_{un,}register_device

Browse Source 
[ Upstream commit 3e3b5dfcd16a3e254aab61bd1e8c417dd4503102 ]

There is a potential UAF between the unregistration routine and the NFC
netlink operations.

The race that cause that UAF can be shown as below:

 (FREE)                      |  (USE)
nfcmrvl_nci_unregister_dev   |  nfc_genl_dev_up
  nci_close_device           |
  nci_unregister_device      |    nfc_get_device
    nfc_unregister_device    |    nfc_dev_up
      rfkill_destory         |
      device_del             |      rfkill_blocked
  ...                        |    ...

The root cause for this race is concluded below:
1. The rfkill_blocked (USE) in nfc_dev_up is supposed to be placed after
the device_is_registered check.
2. Since the netlink operations are possible just after the device_add
in nfc_register_device, the nfc_dev_up() can happen anywhere during the
rfkill creation process, which leads to data race.

This patch reorder these actions to permit
1. Once device_del is finished, the nfc_dev_up cannot dereference the
rfkill object.
2. The rfkill_register need to be placed after the device_add of nfc_dev
because the parent device need to be created first. So this patch keeps
the order but inject device_lock to prevent the data race.

Signed-off-by: Lin Ma <linma@zju.edu.cn>
Fixes: be055b2f89b5 ("NFC: RFKILL support")
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
Link: https://lore.kernel.org/r/20211116152652.19217-1-linma@zju.edu.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Lin Ma 2021-11-16 23:26:52 +08:00 committed by Greg Kroah-Hartman
parent 4a59a36811
commit ff169909ea
1 changed files with 18 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
net/nfc/core.c
View File

@ -106,13 +106,13 @@ int nfc_dev_up(struct nfc_dev *dev)
device_lock(&dev->dev);
if (dev->rfkill && rfkill_blocked(dev->rfkill)) {
rc = -ERFKILL;
if (!device_is_registered(&dev->dev)) {
rc = -ENODEV;
goto error;
}
if (!device_is_registered(&dev->dev)) {
rc = -ENODEV;
if (dev->rfkill && rfkill_blocked(dev->rfkill)) {
rc = -ERFKILL;
goto error;
}
@ -1133,11 +1133,7 @@ int nfc_register_device(struct nfc_dev *dev)
if (rc)
pr_err("Could not register llcp device\n");
rc = nfc_genl_device_added(dev);
if (rc)
pr_debug("The userspace won't be notified that the device %s was added\n",
dev_name(&dev->dev));
device_lock(&dev->dev);
dev->rfkill = rfkill_alloc(dev_name(&dev->dev), &dev->dev,
RFKILL_TYPE_NFC, &nfc_rfkill_ops, dev);
if (dev->rfkill) {
@ -1146,6 +1142,12 @@ int nfc_register_device(struct nfc_dev *dev)
dev->rfkill = NULL;
}
}
device_unlock(&dev->dev);
rc = nfc_genl_device_added(dev);
if (rc)
pr_debug("The userspace won't be notified that the device %s was added\n",
dev_name(&dev->dev));
return 0;
}
@ -1162,10 +1164,17 @@ void nfc_unregister_device(struct nfc_dev *dev)
pr_debug("dev_name=%s\n", dev_name(&dev->dev));
rc = nfc_genl_device_removed(dev);
if (rc)
pr_debug("The userspace won't be notified that the device %s "
"was removed\n", dev_name(&dev->dev));
device_lock(&dev->dev);
if (dev->rfkill) {
rfkill_unregister(dev->rfkill);
rfkill_destroy(dev->rfkill);
}
device_unlock(&dev->dev);
if (dev->ops->check_presence) {
device_lock(&dev->dev);
@ -1175,11 +1184,6 @@ void nfc_unregister_device(struct nfc_dev *dev)
cancel_work_sync(&dev->check_pres_work);
}
rc = nfc_genl_device_removed(dev);
if (rc)
pr_debug("The userspace won't be notified that the device %s "
"was removed\n", dev_name(&dev->dev));
nfc_llcp_unregister_device(dev);
mutex_lock(&nfc_devlist_mutex);