From patchwork Tue Dec 19 17:49:43 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddh Raman Pant X-Patchwork-Id: 13498779 X-Patchwork-Delegate: kuba@kernel.org Received: from sender-of-o51.zoho.in (sender-of-o51.zoho.in [103.117.158.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 961AA39855; Tue, 19 Dec 2023 17:50:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=siddh.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=siddh.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=siddh.me header.i=code@siddh.me header.b="WaX76eR9" ARC-Seal: i=1; a=rsa-sha256; t=1703008193; cv=none; d=zohomail.in; s=zohoarc; b=Nu+mA2iGarrpkhd5ephc4wZ2LYwcN3K8m1swkQkb/LSrEdWz31fm6selUeHBViVmPGQyB2FS6P11g1OhOLVDmnjM06CPQwjRZ/8iWwhDbBnLOdfZqIDmzyJHObSnSha1kg39f1Acc7WWP5I4PigNXQBdcTa/MUxogMmtakQfm8Q= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.in; s=zohoarc; t=1703008193; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=2vugQZgfsUJIq3CJ0ezFnotfB7flJX6f5Obcf5LA+G0=; b=Pyr9hNEDBUl2EgmnKXmTeCjiA2aRg34Dnn/+1F1hrLUJhamll9dTq0BPR5kZyBM/st9kIlufck0LbGDjFEphn0cH0hjh5HzcHVIDyNY2Bi8+Mz4QLAZpT4M1LO6cXTSi97rhRtJjWso0TIx+OXalGpwxHCda2eNTNgf73G49uPI= ARC-Authentication-Results: i=1; mx.zohomail.in; dkim=pass header.i=siddh.me; spf=pass smtp.mailfrom=code@siddh.me; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1703008193; s=zmail; d=siddh.me; i=code@siddh.me; h=From:From:To:To:Cc:Cc:Subject:Subject:Date:Date:Message-ID:In-Reply-To:References:MIME-Version:Content-Transfer-Encoding:Message-Id:Reply-To; bh=2vugQZgfsUJIq3CJ0ezFnotfB7flJX6f5Obcf5LA+G0=; b=WaX76eR9ix37+r5lDzqpJXFi6fbGvsgAKPBvYt7u5G2uGhDw4gu+ESYMVF7nj27a IShXDpeN2E275W7sqWsFiho9ZKfeH3nBD0OngFRP4b9j2r3mfytEtOz++tHJjTYE3MP C3XD2Dvlpxn+UWfSwKM43HpBUGsf4mPzqmNPE7bU= Received: from kampyooter.. (122.170.167.40 [122.170.167.40]) by mx.zoho.in with SMTPS id 1703008191121138.9237515504883; Tue, 19 Dec 2023 23:19:51 +0530 (IST) From: Siddh Raman Pant To: Krzysztof Kozlowski , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, syzbot+bbe84a4010eeea00982d@syzkaller.appspotmail.com, Suman Ghosh Subject: [PATCH net-next v7 1/2] nfc: llcp_core: Hold a ref to llcp_local->dev when holding a ref to llcp_local Date: Tue, 19 Dec 2023 23:19:43 +0530 Message-ID: <376f32d7ea7bac8d1f9310b7749f0e709490e163.1702925869.git.code@siddh.me> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-ZohoMailClient: External X-Patchwork-Delegate: kuba@kernel.org 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@syzkaller.appspotmail.com 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 Signed-off-by: Siddh Raman Pant Reviewed-by: Krzysztof Kozlowski --- net/nfc/llcp_core.c | 39 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c index 1dac28136e6a..18be13fb9b75 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); From patchwork Tue Dec 19 17:49:44 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Siddh Raman Pant X-Patchwork-Id: 13498777 X-Patchwork-Delegate: kuba@kernel.org Received: from sender-of-o51.zoho.in (sender-of-o51.zoho.in [103.117.158.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A7D6E374F8; Tue, 19 Dec 2023 17:50:27 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=siddh.me Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=siddh.me Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=siddh.me header.i=code@siddh.me header.b="d3w5td7+" ARC-Seal: i=1; a=rsa-sha256; t=1703008193; cv=none; d=zohomail.in; s=zohoarc; b=O2lZZ9HwWZENrMMyZrM1Yzod/hHiK+jK7pDwdayn2m/RELjnaOQnoMBhLc8IgMYKPkp/gIlHxPLgIOZ4KmPkAVR/7tlVNneEy+hM1iKd8T4Osi57whn1bSuVC1f4zoc2OMSZ/G0LiBqOEJAOtx4j8NAvNsFbnsoupDURhzgLP5M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.in; s=zohoarc; t=1703008193; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=872tkGPm7VUCxpgjJMERmpcep5wm35gG4ZP3R1Drljw=; b=RAmho9NtBqc8m36Qeb4liQdXtQa96J7exICe0g9G/LAGohQmSHMOq7impzN3sDPMcWQd2iuImnttjpLMgK27s0kTYvj34zoU7mZAl/gvF+mCIki+Z1umMeUvUmt0BcPiigMScbdTp0tfAwvGT47vijAHTqokNzmm7BXo/KMS1F4= ARC-Authentication-Results: i=1; mx.zohomail.in; dkim=pass header.i=siddh.me; spf=pass smtp.mailfrom=code@siddh.me; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1703008193; s=zmail; d=siddh.me; i=code@siddh.me; h=From:From:To:To:Cc:Cc:Subject:Subject:Date:Date:Message-ID:In-Reply-To:References:MIME-Version:Content-Transfer-Encoding:Message-Id:Reply-To; bh=872tkGPm7VUCxpgjJMERmpcep5wm35gG4ZP3R1Drljw=; b=d3w5td7+zQt54GRVeCfbgqH/oeI7Y0/Kqr/EfxRXvdCSbwbJTYN0z51jFOGuQ3DI H+WpJKUkaIMo73xARw10HHgeCqQmna40+cEq7eCkGrH2l7dlq5pCdlaD4A3kptOOgna 5H5BRWJ2kwXXZN9Y9+zP8LcVOSbkCXJBYdTXw+9M= Received: from kampyooter.. (122.170.167.40 [122.170.167.40]) by mx.zoho.in with SMTPS id 1703008191467810.3177351858395; Tue, 19 Dec 2023 23:19:51 +0530 (IST) From: Siddh Raman Pant To: Krzysztof Kozlowski , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Suman Ghosh Subject: [PATCH net-next v7 2/2] nfc: Do not send datagram if socket state isn't LLCP_BOUND Date: Tue, 19 Dec 2023 23:19:44 +0530 Message-ID: <0490c52b68c27730d9916ff22c917b4838c32af4.1702925869.git.code@siddh.me> X-Mailer: git-send-email 2.43.0 In-Reply-To: References: Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-ZohoMailClient: External X-Patchwork-Delegate: kuba@kernel.org As we know we cannot send the datagram (state can be set to LLCP_CLOSED by nfc_llcp_socket_release()), there is no need to proceed further. Thus, bail out early from llcp_sock_sendmsg(). Signed-off-by: Siddh Raman Pant Reviewed-by: Krzysztof Kozlowski Reviewed-by: Suman Ghosh --- net/nfc/llcp_sock.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c index 645677f84dba..819157bbb5a2 100644 --- a/net/nfc/llcp_sock.c +++ b/net/nfc/llcp_sock.c @@ -796,6 +796,11 @@ static int llcp_sock_sendmsg(struct socket *sock, struct msghdr *msg, } if (sk->sk_type == SOCK_DGRAM) { + if (sk->sk_state != LLCP_BOUND) { + release_sock(sk); + return -ENOTCONN; + } + DECLARE_SOCKADDR(struct sockaddr_nfc_llcp *, addr, msg->msg_name);