[linux-nfc] set dev->rfkill to NULL in device cleanup routine

LinMa <[email protected]> Wed, 1 Sep 2021 15:39:36 +0800 (GMT+08:00)
Newsgroups org.01.lists.linux-nfc,dev.linux.lists.oe-linux-nfc
Message-ID <[email protected]>
In nfc_unregister_device() function, the dev->rfkill is forgotten to set to NULL after the rfkill_destroy(). This may lead to possible cocurrency UAF in other functions like nfc_dev_up().



The FREE chain is like


void nfc_unregister_device(struct nfc_dev *dev)
{
  int rc;
  pr_debug("dev_name=%s\n", dev_name(&dev->dev));
  if (dev->rfkill) {
    rfkill_unregister(dev->rfkill);
    rfkill_destroy(dev->rfkill);
  // ......
}



The USE chain is like


static int nfc_genl_dev_up(struct sk_buff *skb, struct genl_info *info)
{
  struct nfc_dev *dev;
  int rc;
  u32 idx;
  if (!info->attrs[NFC_ATTR_DEVICE_INDEX])
    return -EINVAL;
  idx = nla_get_u32(info->attrs[NFC_ATTR_DEVICE_INDEX]);
  dev = nfc_get_device(idx);
  if (!dev)
    return -ENODEV;
  rc = nfc_dev_up(dev);

  // ......
}


int nfc_dev_up(struct nfc_dev *dev)
{
  int rc = 0;
  pr_debug("dev_name=%s\n", dev_name(&dev->dev));
  device_lock(&dev->dev);
  if (dev->rfkill && rfkill_blocked(dev->rfkill)) { // dev->rfkill is not NULL here
    rc = -ERFKILL;
    goto error;
  }
  // ......
}


The FREE chain and USE chain can be like below (as there is no locking protection).


Therefore, the below patch can be added.


Signed-off-by: Lin Ma <[email protected]>
---
 net/nfc/core.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/net/nfc/core.c b/net/nfc/core.c
index 573c80c6ff7a..d0b3224e65d7 100644
--- a/net/nfc/core.c
+++ b/net/nfc/core.c
@@ -1157,6 +1157,7 @@ void nfc_unregister_device(struct nfc_dev *dev)
  if (dev->rfkill) {
  rfkill_unregister(dev->rfkill);
  rfkill_destroy(dev->rfkill);
+ dev->rfkill = NULL;
  }
  if (dev->ops->check_presence) {
--
2.32.0
_______________________________________________
Linux-nfc mailing list -- [email protected]
To unsubscribe send an email to [email protected]
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s