All of lore.kernel.org
 help / color / mirror / Atom feed
From: LinMa <linma@zju.edu.cn>
To: linux-nfc@lists.01.org
Subject: [linux-nfc] set dev->rfkill to NULL in device cleanup routine
Date: Wed, 1 Sep 2021 15:39:36 +0800 (GMT+08:00)	[thread overview]
Message-ID: <5b6649e2.af5bf.17ba04c8d62.Coremail.linma@zju.edu.cn> (raw)

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 <linma@zju.edu.cn>
---
 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 -- linux-nfc@lists.01.org
To unsubscribe send an email to linux-nfc-leave@lists.01.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

WARNING: multiple messages have this Message-ID (diff)
From: LinMa <linma@zju.edu.cn>
To: linux-nfc@lists.01.org
Subject: set dev->rfkill to NULL in device cleanup routine
Date: Wed, 01 Sep 2021 07:39:43 +0000	[thread overview]
Message-ID: <5b6649e2.af5bf.17ba04c8d62.Coremail.linma@zju.edu.cn> (raw)

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

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 <linma@zju.edu.cn>
---
 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

             reply	other threads:[~2021-09-01  7:39 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-01  7:39 LinMa [this message]
2021-09-01  7:39 ` set dev->rfkill to NULL in device cleanup routine LinMa
2021-09-01  8:27 ` [linux-nfc] " Krzysztof Kozlowski
2021-09-01  8:27   ` Krzysztof Kozlowski
2021-09-01  8:27   ` [linux-nfc] " Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b6649e2.af5bf.17ba04c8d62.Coremail.linma@zju.edu.cn \
    --to=linma@zju.edu.cn \
    --cc=linux-nfc@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.