lkml.org 
[lkml]   [2021]   [Nov]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 186/207] NFC: reorder the logic in nfc_{un,}register_device
    Date
    From: Lin Ma <linma@zju.edu.cn>

    [ 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>
    ---
    net/nfc/core.c | 32 ++++++++++++++++++--------------
    1 file changed, 18 insertions(+), 14 deletions(-)

    diff --git a/net/nfc/core.c b/net/nfc/core.c
    index c699d64a0753a..32a2dfc08f480 100644
    --- a/net/nfc/core.c
    +++ b/net/nfc/core.c
    @@ -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);
    --
    2.33.0


    \
     
     \ /
      Last update: 2021-11-24 13:29    [W:4.067 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site