Patch "nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local" has been added to the 6.1-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local

to the 6.1-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     nfc-llcp_core-hold-a-ref-to-llcp_local-dev-when-hold.patch
and it can be found in the queue-6.1 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 931ea9a2205ca793f1dcdff5f7f215cc9d0f2826
Author: Siddh Raman Pant <code@xxxxxxxx>
Date:   Tue Dec 19 23:19:43 2023 +0530

    nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local
    
    [ Upstream commit c95f919567d6f1914f13350af61a1b044ac85014 ]
    
    llcp_sock_sendmsg() calls nfc_llcp_send_ui_frame() which in turn calls
    nfc_alloc_send_skb(), which accesses the nfc_dev from the llcp_sock for
    getting the headroom and tailroom needed for skb allocation.
    
    Parallelly the nfc_dev can be freed, as the refcount is decreased via
    nfc_free_device(), leading to a UAF reported by Syzkaller, which can
    be summarized as follows:
    
    (1) llcp_sock_sendmsg() -> nfc_llcp_send_ui_frame()
            -> nfc_alloc_send_skb() -> Dereference *nfc_dev
    (2) virtual_ncidev_close() -> nci_free_device() -> nfc_free_device()
            -> put_device() -> nfc_release() -> Free *nfc_dev
    
    When a reference to llcp_local is acquired, we do not acquire the same
    for the nfc_dev. This leads to freeing even when the llcp_local is in
    use, and this is the case with the UAF described above too.
    
    Thus, when we acquire a reference to llcp_local, we should acquire a
    reference to nfc_dev, and release the references appropriately later.
    
    References for llcp_local is initialized in nfc_llcp_register_device()
    (which is called by nfc_register_device()). Thus, we should acquire a
    reference to nfc_dev there.
    
    nfc_unregister_device() calls nfc_llcp_unregister_device() which in
    turn calls nfc_llcp_local_put(). Thus, the reference to nfc_dev is
    appropriately released later.
    
    Reported-and-tested-by: syzbot+bbe84a4010eeea00982d@xxxxxxxxxxxxxxxxxxxxxxxxx
    Closes: https://syzkaller.appspot.com/bug?extid=bbe84a4010eeea00982d
    Fixes: c7aa12252f51 ("NFC: Take a reference on the LLCP local pointer when creating a socket")
    Reviewed-by: Suman Ghosh <sumang@xxxxxxxxxxx>
    Signed-off-by: Siddh Raman Pant <code@xxxxxxxx>
    Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
    Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 1dac28136e6a3..18be13fb9b75a 100644
--- a/net/nfc/llcp_core.c
+++ b/net/nfc/llcp_core.c
@@ -145,6 +145,13 @@ static void nfc_llcp_socket_release(struct nfc_llcp_local *local, bool device,
 
 static struct nfc_llcp_local *nfc_llcp_local_get(struct nfc_llcp_local *local)
 {
+	/* Since using nfc_llcp_local may result in usage of nfc_dev, whenever
+	 * we hold a reference to local, we also need to hold a reference to
+	 * the device to avoid UAF.
+	 */
+	if (!nfc_get_device(local->dev->idx))
+		return NULL;
+
 	kref_get(&local->ref);
 
 	return local;
@@ -177,10 +184,18 @@ static void local_release(struct kref *ref)
 
 int nfc_llcp_local_put(struct nfc_llcp_local *local)
 {
+	struct nfc_dev *dev;
+	int ret;
+
 	if (local == NULL)
 		return 0;
 
-	return kref_put(&local->ref, local_release);
+	dev = local->dev;
+
+	ret = kref_put(&local->ref, local_release);
+	nfc_put_device(dev);
+
+	return ret;
 }
 
 static struct nfc_llcp_sock *nfc_llcp_sock_get(struct nfc_llcp_local *local,
@@ -959,8 +974,17 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
 	}
 
 	new_sock = nfc_llcp_sock(new_sk);
-	new_sock->dev = local->dev;
+
 	new_sock->local = nfc_llcp_local_get(local);
+	if (!new_sock->local) {
+		reason = LLCP_DM_REJ;
+		sock_put(&new_sock->sk);
+		release_sock(&sock->sk);
+		sock_put(&sock->sk);
+		goto fail;
+	}
+
+	new_sock->dev = local->dev;
 	new_sock->rw = sock->rw;
 	new_sock->miux = sock->miux;
 	new_sock->nfc_protocol = sock->nfc_protocol;
@@ -1597,7 +1621,16 @@ int nfc_llcp_register_device(struct nfc_dev *ndev)
 	if (local == NULL)
 		return -ENOMEM;
 
-	local->dev = ndev;
+	/* As we are going to initialize local's refcount, we need to get the
+	 * nfc_dev to avoid UAF, otherwise there is no point in continuing.
+	 * See nfc_llcp_local_get().
+	 */
+	local->dev = nfc_get_device(ndev->idx);
+	if (!local->dev) {
+		kfree(local);
+		return -ENODEV;
+	}
+
 	INIT_LIST_HEAD(&local->list);
 	kref_init(&local->ref);
 	mutex_init(&local->sdp_lock);




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux