From: Fedor Pchelkin <pchelkin@ispras.ru>
To: Oleksij Rempel <linux@rempel-privat.de>
Cc: Fedor Pchelkin <pchelkin@ispras.ru>,
Marc Kleine-Budde <mkl@pengutronix.de>,
kernel@pengutronix.de, Robin van der Gracht <robin@protonic.nl>,
Oliver Hartkopp <socketcan@hartkopp.net>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Kurt Van Dijck <dev.kurt@vandijck-laurijssen.be>,
linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
Alexey Khoroshilov <khoroshilov@ispras.ru>,
lvc-project@linuxtesting.org
Subject: [PATCH 2/2] can: j1939: avoid possible use-after-free when j1939_can_rx_register fails
Date: Fri, 26 May 2023 20:19:10 +0300 [thread overview]
Message-ID: <20230526171910.227615-3-pchelkin@ispras.ru> (raw)
In-Reply-To: <20230526171910.227615-1-pchelkin@ispras.ru>
Syzkaller reports the following failure:
BUG: KASAN: use-after-free in kref_put include/linux/kref.h:64 [inline]
BUG: KASAN: use-after-free in j1939_priv_put+0x25/0xa0 net/can/j1939/main.c:172
Write of size 4 at addr ffff888141c15058 by task swapper/3/0
CPU: 3 PID: 0 Comm: swapper/3 Not tainted 5.10.144-syzkaller #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x107/0x167 lib/dump_stack.c:118
print_address_description.constprop.0+0x1c/0x220 mm/kasan/report.c:385
__kasan_report mm/kasan/report.c:545 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:562
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x145/0x190 mm/kasan/generic.c:192
instrument_atomic_read_write include/linux/instrumented.h:101 [inline]
atomic_fetch_sub_release include/asm-generic/atomic-instrumented.h:220 [inline]
__refcount_sub_and_test include/linux/refcount.h:272 [inline]
__refcount_dec_and_test include/linux/refcount.h:315 [inline]
refcount_dec_and_test include/linux/refcount.h:333 [inline]
kref_put include/linux/kref.h:64 [inline]
j1939_priv_put+0x25/0xa0 net/can/j1939/main.c:172
j1939_sk_sock_destruct+0x44/0x90 net/can/j1939/socket.c:374
__sk_destruct+0x4e/0x820 net/core/sock.c:1784
rcu_do_batch kernel/rcu/tree.c:2485 [inline]
rcu_core+0xb35/0x1a30 kernel/rcu/tree.c:2726
__do_softirq+0x289/0x9a3 kernel/softirq.c:298
asm_call_irq_on_stack+0x12/0x20
</IRQ>
__run_on_irqstack arch/x86/include/asm/irq_stack.h:26 [inline]
run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:77 [inline]
do_softirq_own_stack+0xaa/0xe0 arch/x86/kernel/irq_64.c:77
invoke_softirq kernel/softirq.c:393 [inline]
__irq_exit_rcu kernel/softirq.c:423 [inline]
irq_exit_rcu+0x136/0x200 kernel/softirq.c:435
sysvec_apic_timer_interrupt+0x4d/0x100 arch/x86/kernel/apic/apic.c:1095
asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:635
Allocated by task 1141:
kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
kasan_set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc.constprop.0+0xc9/0xd0 mm/kasan/common.c:461
kmalloc include/linux/slab.h:552 [inline]
kzalloc include/linux/slab.h:664 [inline]
j1939_priv_create net/can/j1939/main.c:131 [inline]
j1939_netdev_start+0x111/0x860 net/can/j1939/main.c:268
j1939_sk_bind+0x8ea/0xd30 net/can/j1939/socket.c:485
__sys_bind+0x1f2/0x260 net/socket.c:1645
__do_sys_bind net/socket.c:1656 [inline]
__se_sys_bind net/socket.c:1654 [inline]
__x64_sys_bind+0x6f/0xb0 net/socket.c:1654
do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x61/0xc6
Freed by task 1141:
kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48
kasan_set_track+0x1c/0x30 mm/kasan/common.c:56
kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:355
__kasan_slab_free+0x112/0x170 mm/kasan/common.c:422
slab_free_hook mm/slub.c:1542 [inline]
slab_free_freelist_hook+0xad/0x190 mm/slub.c:1576
slab_free mm/slub.c:3149 [inline]
kfree+0xd9/0x3b0 mm/slub.c:4125
j1939_netdev_start+0x5ee/0x860 net/can/j1939/main.c:300
j1939_sk_bind+0x8ea/0xd30 net/can/j1939/socket.c:485
__sys_bind+0x1f2/0x260 net/socket.c:1645
__do_sys_bind net/socket.c:1656 [inline]
__se_sys_bind net/socket.c:1654 [inline]
__x64_sys_bind+0x6f/0xb0 net/socket.c:1654
do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x61/0xc6
It can be caused by this scenario:
CPU0 CPU1
j1939_sk_bind(socket0, ndev0, ...)
j1939_netdev_start()
j1939_sk_bind(socket1, ndev0, ...)
j1939_netdev_start()
mutex_lock(&j1939_netdev_lock)
j1939_priv_set(ndev0, priv)
mutex_unlock(&j1939_netdev_lock)
if (priv_new)
kref_get(&priv_new->rx_kref)
return priv_new;
/* inside j1939_sk_bind() */
jsk->priv = priv
j1939_can_rx_register(priv) // fails
j1939_priv_set(ndev, NULL)
kfree(priv)
j1939_sk_sock_destruct()
j1939_priv_put() // <- uaf
To avoid this, call j1939_can_rx_register() under j1939_netdev_lock so
that a concurrent thread cannot process j1939_priv before
j1939_can_rx_register() returns.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
net/can/j1939/main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/can/j1939/main.c b/net/can/j1939/main.c
index 6ed79afe19a5..ecff1c947d68 100644
--- a/net/can/j1939/main.c
+++ b/net/can/j1939/main.c
@@ -290,16 +290,18 @@ struct j1939_priv *j1939_netdev_start(struct net_device *ndev)
return priv_new;
}
j1939_priv_set(ndev, priv);
- mutex_unlock(&j1939_netdev_lock);
ret = j1939_can_rx_register(priv);
if (ret < 0)
goto out_priv_put;
+ mutex_unlock(&j1939_netdev_lock);
return priv;
out_priv_put:
j1939_priv_set(ndev, NULL);
+ mutex_unlock(&j1939_netdev_lock);
+
dev_put(ndev);
kfree(priv);
--
2.34.1
next prev parent reply other threads:[~2023-05-26 17:20 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-26 17:19 [PATCH 0/2] can: j1939: avoid possible use-after-free when j1939_can_rx_register fails Fedor Pchelkin
2023-05-26 17:19 ` [PATCH 1/2] can: j1939: change j1939_netdev_lock type to mutex Fedor Pchelkin
2023-06-02 12:33 ` Oleksij Rempel
2023-05-26 17:19 ` Fedor Pchelkin [this message]
2023-05-26 18:15 ` [PATCH 2/2] can: j1939: avoid possible use-after-free when j1939_can_rx_register fails Oleksij Rempel
2023-05-26 18:50 ` Fedor Pchelkin
2023-05-27 5:57 ` Oleksij Rempel
2023-05-27 10:05 ` Fedor Pchelkin
2023-06-02 12:35 ` Oleksij Rempel
2023-06-02 16:06 ` Fedor Pchelkin
2023-06-05 6:37 ` [PATCH 0/2] " Marc Kleine-Budde
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=20230526171910.227615-3-pchelkin@ispras.ru \
--to=pchelkin@ispras.ru \
--cc=davem@davemloft.net \
--cc=dev.kurt@vandijck-laurijssen.be \
--cc=edumazet@google.com \
--cc=kernel@pengutronix.de \
--cc=khoroshilov@ispras.ru \
--cc=kuba@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rempel-privat.de \
--cc=lvc-project@linuxtesting.org \
--cc=mkl@pengutronix.de \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=robin@protonic.nl \
--cc=socketcan@hartkopp.net \
/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.