diff mbox series

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

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

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 21 this patch: 21
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 21 this patch: 21
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Ziyang Xuan (William) March 30, 2023, 1:25 a.m. UTC
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(+)

Comments

patchwork-bot+netdevbpf@kernel.org March 31, 2023, 8:20 a.m. UTC | #1
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 mbox series

Patch

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;
 }