From d5ee2ff98322337951c56398e79d51815acbf955 Mon Sep 17 00:00:00 2001 From: Manivannan Sadhasivam Date: Thu, 9 Apr 2026 23:04:12 +0530 Subject: net: qrtr: ns: Limit the maximum server registration per node Current code does no bound checking on the number of servers added per node. A malicious client can flood NEW_SERVER messages and exhaust memory. Fix this issue by limiting the maximum number of server registrations to 256 per node. If the NEW_SERVER message is received for an old port, then don't restrict it as it will get replaced. While at it, also rate limit the error messages in the failure path of qrtr_ns_worker(). Note that the limit of 256 is chosen based on the current platform requirements. If requirement changes in the future, this limit can be increased. Cc: stable@vger.kernel.org Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Reported-by: Yiming Qian Reviewed-by: Simon Horman Signed-off-by: Manivannan Sadhasivam Link: https://patch.msgid.link/20260409-qrtr-fix-v3-1-00a8a5ff2b51@oss.qualcomm.com Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 3203b2220860..63cb5861d87a 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -67,8 +67,14 @@ struct qrtr_server { struct qrtr_node { unsigned int id; struct xarray servers; + u32 server_count; }; +/* Max server limit is chosen based on the current platform requirements. If the + * requirement changes in the future, this value can be increased. + */ +#define QRTR_NS_MAX_SERVERS 256 + static struct qrtr_node *node_get(unsigned int node_id) { struct qrtr_node *node; @@ -229,6 +235,17 @@ static struct qrtr_server *server_add(unsigned int service, if (!service || !port) return NULL; + node = node_get(node_id); + if (!node) + return NULL; + + /* Make sure the new servers per port are capped at the maximum value */ + old = xa_load(&node->servers, port); + if (!old && node->server_count >= QRTR_NS_MAX_SERVERS) { + pr_err_ratelimited("QRTR client node %u exceeds max server limit!\n", node_id); + return NULL; + } + srv = kzalloc_obj(*srv); if (!srv) return NULL; @@ -238,10 +255,6 @@ static struct qrtr_server *server_add(unsigned int service, srv->node = node_id; srv->port = port; - node = node_get(node_id); - if (!node) - goto err; - /* Delete the old server on the same port */ old = xa_store(&node->servers, port, srv, GFP_KERNEL); if (old) { @@ -252,6 +265,8 @@ static struct qrtr_server *server_add(unsigned int service, } else { kfree(old); } + } else { + node->server_count++; } trace_qrtr_ns_server_add(srv->service, srv->instance, @@ -292,6 +307,7 @@ static int server_del(struct qrtr_node *node, unsigned int port, bool bcast) } kfree(srv); + node->server_count--; return 0; } @@ -670,7 +686,7 @@ static void qrtr_ns_worker(struct work_struct *work) } if (ret < 0) - pr_err("failed while handling packet from %d:%d", + pr_err_ratelimited("failed while handling packet from %d:%d", sq.sq_node, sq.sq_port); } -- cgit v1.2.3 From 5640227d9a21c6a8be249a10677b832e7f40dc55 Mon Sep 17 00:00:00 2001 From: Manivannan Sadhasivam Date: Thu, 9 Apr 2026 23:04:13 +0530 Subject: net: qrtr: ns: Limit the maximum number of lookups Current code does no bound checking on the number of lookups a client can perform. Though the code restricts the lookups to local clients, there is still a possibility of a malicious local client sending a flood of NEW_LOOKUP messages over the same socket. Fix this issue by limiting the maximum number of lookups to 64 globally. Since the nameserver allows only atmost one local observer, this global lookup count will ensure that the lookups stay within the limit. Note that, limit of 64 is chosen based on the current platform requirements. If requirement changes in the future, this limit can be increased. Cc: stable@vger.kernel.org Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Signed-off-by: Manivannan Sadhasivam Link: https://patch.msgid.link/20260409-qrtr-fix-v3-2-00a8a5ff2b51@oss.qualcomm.com Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 63cb5861d87a..5b08d4d4840a 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -22,6 +22,7 @@ static struct { struct socket *sock; struct sockaddr_qrtr bcast_sq; struct list_head lookups; + u32 lookup_count; struct workqueue_struct *workqueue; struct work_struct work; int local_node; @@ -70,10 +71,11 @@ struct qrtr_node { u32 server_count; }; -/* Max server limit is chosen based on the current platform requirements. If the - * requirement changes in the future, this value can be increased. +/* Max server, lookup limits are chosen based on the current platform requirements. + * If the requirement changes in the future, these values can be increased. */ #define QRTR_NS_MAX_SERVERS 256 +#define QRTR_NS_MAX_LOOKUPS 64 static struct qrtr_node *node_get(unsigned int node_id) { @@ -433,6 +435,7 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from, list_del(&lookup->li); kfree(lookup); + qrtr_ns.lookup_count--; } /* Remove the server belonging to this port but don't broadcast @@ -550,6 +553,11 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from, if (from->sq_node != qrtr_ns.local_node) return -EINVAL; + if (qrtr_ns.lookup_count >= QRTR_NS_MAX_LOOKUPS) { + pr_err_ratelimited("QRTR client node exceeds max lookup limit!\n"); + return -ENOSPC; + } + lookup = kzalloc_obj(*lookup); if (!lookup) return -ENOMEM; @@ -558,6 +566,7 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from, lookup->service = service; lookup->instance = instance; list_add_tail(&lookup->li, &qrtr_ns.lookups); + qrtr_ns.lookup_count++; memset(&filter, 0, sizeof(filter)); filter.service = service; @@ -598,6 +607,7 @@ static void ctrl_cmd_del_lookup(struct sockaddr_qrtr *from, list_del(&lookup->li); kfree(lookup); + qrtr_ns.lookup_count--; } } -- cgit v1.2.3 From 68efba36446a7774ea5b971257ade049272a07ac Mon Sep 17 00:00:00 2001 From: Manivannan Sadhasivam Date: Thu, 9 Apr 2026 23:04:14 +0530 Subject: net: qrtr: ns: Free the node during ctrl_cmd_bye() A node sends the BYE packet when it is about to go down. So the nameserver should advertise the removal of the node to all remote and local observers and free the node finally. But currently, the nameserver doesn't free the node memory even after processing the BYE packet. This causes the node memory to leak. Hence, remove the node from Xarray list and free the node memory during both success and failure case of ctrl_cmd_bye(). Cc: stable@vger.kernel.org Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Signed-off-by: Manivannan Sadhasivam Link: https://patch.msgid.link/20260409-qrtr-fix-v3-3-00a8a5ff2b51@oss.qualcomm.com Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 5b08d4d4840a..1b9a90240a68 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -359,7 +359,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from) struct qrtr_node *node; unsigned long index; struct kvec iv; - int ret; + int ret = 0; iv.iov_base = &pkt; iv.iov_len = sizeof(pkt); @@ -374,8 +374,10 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from) /* Advertise the removal of this client to all local servers */ local_node = node_get(qrtr_ns.local_node); - if (!local_node) - return 0; + if (!local_node) { + ret = 0; + goto delete_node; + } memset(&pkt, 0, sizeof(pkt)); pkt.cmd = cpu_to_le32(QRTR_TYPE_BYE); @@ -392,10 +394,18 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from) ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); if (ret < 0 && ret != -ENODEV) { pr_err("failed to send bye cmd\n"); - return ret; + goto delete_node; } } - return 0; + + /* Ignore -ENODEV */ + ret = 0; + +delete_node: + xa_erase(&nodes, from->sq_node); + kfree(node); + + return ret; } static int ctrl_cmd_del_client(struct sockaddr_qrtr *from, -- cgit v1.2.3 From 27d5e84e810b0849d08b9aec68e48570461ce313 Mon Sep 17 00:00:00 2001 From: Manivannan Sadhasivam Date: Thu, 9 Apr 2026 23:04:15 +0530 Subject: net: qrtr: ns: Limit the total number of nodes Currently, the nameserver doesn't limit the number of nodes it handles. This can be an attack vector if a malicious client starts registering random nodes, leading to memory exhaustion. Hence, limit the maximum number of nodes to 64. Note that, limit of 64 is chosen based on the current platform requirements. If requirement changes in the future, this limit can be increased. Cc: stable@vger.kernel.org Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Signed-off-by: Manivannan Sadhasivam Link: https://patch.msgid.link/20260409-qrtr-fix-v3-4-00a8a5ff2b51@oss.qualcomm.com Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 1b9a90240a68..c0418764470b 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -71,12 +71,16 @@ struct qrtr_node { u32 server_count; }; -/* Max server, lookup limits are chosen based on the current platform requirements. - * If the requirement changes in the future, these values can be increased. +/* Max nodes, server, lookup limits are chosen based on the current platform + * requirements. If the requirement changes in the future, these values can be + * increased. */ +#define QRTR_NS_MAX_NODES 64 #define QRTR_NS_MAX_SERVERS 256 #define QRTR_NS_MAX_LOOKUPS 64 +static u8 node_count; + static struct qrtr_node *node_get(unsigned int node_id) { struct qrtr_node *node; @@ -85,6 +89,11 @@ static struct qrtr_node *node_get(unsigned int node_id) if (node) return node; + if (node_count >= QRTR_NS_MAX_NODES) { + pr_err_ratelimited("QRTR clients exceed max node limit!\n"); + return NULL; + } + /* If node didn't exist, allocate and insert it to the tree */ node = kzalloc_obj(*node); if (!node) @@ -98,6 +107,8 @@ static struct qrtr_node *node_get(unsigned int node_id) return NULL; } + node_count++; + return node; } @@ -404,6 +415,7 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from) delete_node: xa_erase(&nodes, from->sq_node); kfree(node); + node_count--; return ret; } -- cgit v1.2.3 From 7809fea20c9404bfcfa6112ec08d1fe1d3520beb Mon Sep 17 00:00:00 2001 From: Manivannan Sadhasivam Date: Thu, 9 Apr 2026 23:04:16 +0530 Subject: net: qrtr: ns: Fix use-after-free in driver remove() In the remove callback, if a packet arrives after destroy_workqueue() is called, but before sock_release(), the qrtr_ns_data_ready() callback will try to queue the work, causing use-after-free issue. Fix this issue by saving the default 'sk_data_ready' callback during qrtr_ns_init() and use it to replace the qrtr_ns_data_ready() callback at the start of remove(). This ensures that even if a packet arrives after destroy_workqueue(), the work struct will not be dereferenced. Note that it is also required to ensure that the RX threads are completed before destroying the workqueue, because the threads could be using the qrtr_ns_data_ready() callback. Cc: stable@vger.kernel.org Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Signed-off-by: Manivannan Sadhasivam Link: https://patch.msgid.link/20260409-qrtr-fix-v3-5-00a8a5ff2b51@oss.qualcomm.com Signed-off-by: Jakub Kicinski --- net/qrtr/ns.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index c0418764470b..b3f9bbcf9ab9 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -25,6 +25,7 @@ static struct { u32 lookup_count; struct workqueue_struct *workqueue; struct work_struct work; + void (*saved_data_ready)(struct sock *sk); int local_node; } qrtr_ns; @@ -757,6 +758,7 @@ int qrtr_ns_init(void) goto err_sock; } + qrtr_ns.saved_data_ready = qrtr_ns.sock->sk->sk_data_ready; qrtr_ns.sock->sk->sk_data_ready = qrtr_ns_data_ready; sq.sq_port = QRTR_PORT_CTRL; @@ -797,6 +799,10 @@ int qrtr_ns_init(void) return 0; err_wq: + write_lock_bh(&qrtr_ns.sock->sk->sk_callback_lock); + qrtr_ns.sock->sk->sk_data_ready = qrtr_ns.saved_data_ready; + write_unlock_bh(&qrtr_ns.sock->sk->sk_callback_lock); + destroy_workqueue(qrtr_ns.workqueue); err_sock: sock_release(qrtr_ns.sock); @@ -806,7 +812,12 @@ EXPORT_SYMBOL_GPL(qrtr_ns_init); void qrtr_ns_remove(void) { + write_lock_bh(&qrtr_ns.sock->sk->sk_callback_lock); + qrtr_ns.sock->sk->sk_data_ready = qrtr_ns.saved_data_ready; + write_unlock_bh(&qrtr_ns.sock->sk->sk_callback_lock); + cancel_work_sync(&qrtr_ns.work); + synchronize_net(); destroy_workqueue(qrtr_ns.workqueue); /* sock_release() expects the two references that were put during -- cgit v1.2.3