2023-03-30 01:53:17

by Ziyang Xuan (William)

Subject: [PATCH net] net: qrtr: Fix a refcount bug in qrtr_recvmsg()

Syzbot reported a bug as following:

refcount_t: addition on 0; use-after-free.
...
RIP: 0010:refcount_warn_saturate+0x17c/0x1f0 lib/refcount.c:25
...
Call Trace:
<TASK>
__refcount_add include/linux/refcount.h:199 [inline]
__refcount_inc include/linux/refcount.h:250 [inline]
refcount_inc include/linux/refcount.h:267 [inline]
kref_get include/linux/kref.h:45 [inline]
qrtr_node_acquire net/qrtr/af_qrtr.c:202 [inline]
qrtr_node_lookup net/qrtr/af_qrtr.c:398 [inline]
qrtr_send_resume_tx net/qrtr/af_qrtr.c:1003 [inline]
qrtr_recvmsg+0x85f/0x990 net/qrtr/af_qrtr.c:1070
sock_recvmsg_nosec net/socket.c:1017 [inline]
sock_recvmsg+0xe2/0x160 net/socket.c:1038
qrtr_ns_worker+0x170/0x1700 net/qrtr/ns.c:688
process_one_work+0x991/0x15c0 kernel/workqueue.c:2390
worker_thread+0x669/0x1090 kernel/workqueue.c:2537

It occurs in the concurrent scenario of qrtr_recvmsg() and
qrtr_endpoint_unregister() as following:

cpu0 cpu1
qrtr_recvmsg qrtr_endpoint_unregister
qrtr_send_resume_tx qrtr_node_release
qrtr_node_lookup mutex_lock(&qrtr_node_lock)
spin_lock_irqsave(&qrtr_nodes_lock, ) refcount_dec_and_test(&node->ref) [node->ref == 0]
radix_tree_lookup [node != NULL] __qrtr_node_release
qrtr_node_acquire spin_lock_irqsave(&qrtr_nodes_lock, )
kref_get(&node->ref) [WARNING] ...
mutex_unlock(&qrtr_node_lock)

Use qrtr_node_lock to protect qrtr_node_lookup() implementation, this
is actually improving the protection of node reference.

Fixes: 0a7e0d0ef054 ("net: qrtr: Migrate node lookup tree to spinlock")
Reported-by: [email protected]
Link: https://syzkaller.appspot.com/bug?extid=a7492efaa5d61b51db23
Signed-off-by: Ziyang Xuan <[email protected]>
---
net/qrtr/af_qrtr.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 5c2fb992803b..3a70255c8d02 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -393,10 +393,12 @@ static struct qrtr_node *qrtr_node_lookup(unsigned int nid)
struct qrtr_node *node;
unsigned long flags;

+ mutex_lock(&qrtr_node_lock);
spin_lock_irqsave(&qrtr_nodes_lock, flags);
node = radix_tree_lookup(&qrtr_nodes, nid);
node = qrtr_node_acquire(node);
spin_unlock_irqrestore(&qrtr_nodes_lock, flags);
+ mutex_unlock(&qrtr_node_lock);

return node;
}
--
2.25.1

2023-03-31 08:31:58

by patchwork-bot+netdevbpf

Subject: Re: [PATCH net] net: qrtr: Fix a refcount bug in qrtr_recvmsg()

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Thu, 30 Mar 2023 09:25:32 +0800 you wrote:
> Syzbot reported a bug as following:
>
> refcount_t: addition on 0; use-after-free.
> ...
> RIP: 0010:refcount_warn_saturate+0x17c/0x1f0 lib/refcount.c:25
> ...
> Call Trace:
> <TASK>
> __refcount_add include/linux/refcount.h:199 [inline]
> __refcount_inc include/linux/refcount.h:250 [inline]
> refcount_inc include/linux/refcount.h:267 [inline]
> kref_get include/linux/kref.h:45 [inline]
> qrtr_node_acquire net/qrtr/af_qrtr.c:202 [inline]
> qrtr_node_lookup net/qrtr/af_qrtr.c:398 [inline]
> qrtr_send_resume_tx net/qrtr/af_qrtr.c:1003 [inline]
> qrtr_recvmsg+0x85f/0x990 net/qrtr/af_qrtr.c:1070
> sock_recvmsg_nosec net/socket.c:1017 [inline]
> sock_recvmsg+0xe2/0x160 net/socket.c:1038
> qrtr_ns_worker+0x170/0x1700 net/qrtr/ns.c:688
> process_one_work+0x991/0x15c0 kernel/workqueue.c:2390
> worker_thread+0x669/0x1090 kernel/workqueue.c:2537
>
> [...]

Here is the summary with links:
- [net] net: qrtr: Fix a refcount bug in qrtr_recvmsg()
https://git.kernel.org/netdev/net/c/44d807320000

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html