| Message ID | 20230330012532.1065159-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 44d807320000db0d0013372ad39b53e12d52f758 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: qrtr: Fix a refcount bug in qrtr_recvmsg() | expand |
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: 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!
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; }
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: syzbot+a7492efaa5d61b51db23@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=a7492efaa5d61b51db23 Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- net/qrtr/af_qrtr.c | 2 ++ 1 file changed, 2 insertions(+)