#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
---
net/nfc/llcp_core.c | 72 +++++++++++++++++++++++++++++----------------
1 file changed, 47 insertions(+), 25 deletions(-)
diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c
index 1dac28136e6a..2f77200a3720 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,
@@ -901,7 +916,7 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
if (dsap != LLCP_SAP_SDP) {
sock = nfc_llcp_sock_get(local, dsap, LLCP_SAP_SDP);
- if (sock == NULL || sock->sk.sk_state != LLCP_LISTEN) {
+ if (!sock || sock->sk.sk_state != LLCP_LISTEN) {
reason = LLCP_DM_NOBOUND;
goto fail;
}
@@ -910,7 +925,7 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
size_t sn_len;
sn = nfc_llcp_connect_sn(skb, &sn_len);
- if (sn == NULL) {
+ if (!sn) {
reason = LLCP_DM_NOBOUND;
goto fail;
}
@@ -918,7 +933,7 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
pr_debug("Service name length %zu\n", sn_len);
sock = nfc_llcp_sock_get_sn(local, sn, sn_len);
- if (sock == NULL) {
+ if (!sock) {
reason = LLCP_DM_NOBOUND;
goto fail;
}
@@ -928,39 +943,31 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
parent = &sock->sk;
- if (sk_acceptq_is_full(parent)) {
- reason = LLCP_DM_REJ;
- release_sock(&sock->sk);
- sock_put(&sock->sk);
- goto fail;
- }
+ if (sk_acceptq_is_full(parent))
+ goto fail_put_sock;
if (sock->ssap == LLCP_SDP_UNBOUND) {
u8 ssap = nfc_llcp_reserve_sdp_ssap(local);
pr_debug("First client, reserving %d\n", ssap);
- if (ssap == LLCP_SAP_MAX) {
- reason = LLCP_DM_REJ;
- release_sock(&sock->sk);
- sock_put(&sock->sk);
- goto fail;
- }
+ if (ssap == LLCP_SAP_MAX)
+ goto fail_put_sock;
sock->ssap = ssap;
}
new_sk = nfc_llcp_sock_alloc(NULL, parent->sk_type, GFP_ATOMIC, 0);
- if (new_sk == NULL) {
- reason = LLCP_DM_REJ;
- release_sock(&sock->sk);
- sock_put(&sock->sk);
- goto fail;
- }
+ if (!new_sk)
+ goto fail_put_sock;
new_sock = nfc_llcp_sock(new_sk);
- new_sock->dev = local->dev;
+
new_sock->local = nfc_llcp_local_get(local);
+ if (!new_sock->local)
+ goto fail_free_new_sock;
+
+ new_sock->dev = local->dev;
new_sock->rw = sock->rw;
new_sock->miux = sock->miux;
new_sock->nfc_protocol = sock->nfc_protocol;
@@ -1004,8 +1011,14 @@ static void nfc_llcp_recv_connect(struct nfc_llcp_local *local,
return;
+fail_free_new_sock:
+ sock_put(&new_sock->sk);
+ nfc_llcp_sock_free(new_sock);
+fail_put_sock:
+ reason = LLCP_DM_REJ;
+ release_sock(&sock->sk);
+ sock_put(&sock->sk);
fail:
- /* Send DM */
nfc_llcp_send_dm(local, dsap, ssap, reason);
}
@@ -1597,7 +1610,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);
--
2.42.0
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: [email protected]
Tested on:
commit: 26aff849 Merge tag 'bcachefs-2023-12-10' of https://ev..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
console output: https://syzkaller.appspot.com/x/log.txt?x=110579b6e80000
kernel config: https://syzkaller.appspot.com/x/.config?x=68704e9af8d2dd7b
dashboard link: https://syzkaller.appspot.com/bug?extid=bbe84a4010eeea00982d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1059a15ce80000
Note: testing is done by a robot and is best-effort only.