[PATCH 0/2] net: qrtr: ns: Fix unbounded server/lookup messages

[PATCH 0/2] net: qrtr: ns: Fix unbounded server/lookup messages

[Manivannan Sadhasivam]

Hi,

This series fixes a couple of possible memory exhaustion issues when the
nameservice driver receives a flood of QRTR messages.

Manivannan Sadhasivam (2):
  net: qrtr: ns: Limit the maximum server registration per node
  net: qrtr: ns: Limit the maximum lookups per socket

 net/qrtr/ns.c | 38 ++++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

-- 
2.51.0

[PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node

[Manivannan Sadhasivam]

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.

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 <yimingqian591@gmail.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 net/qrtr/ns.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index 3203b2220860..fb4e8a2d370d 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;
 }
-- 
2.51.0

[PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket

[Manivannan Sadhasivam]

Current code does no bound checking on the number of lookups a client can
perform per socket. 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 per socket.
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 <manivannan.sadhasivam@oss.qualcomm.com>
---
 net/qrtr/ns.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index fb4e8a2d370d..707fde809939 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -70,10 +70,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)
 {
@@ -545,11 +546,24 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from,
 	struct qrtr_node *node;
 	unsigned long node_idx;
 	unsigned long srv_idx;
+	u8 count = 0;
 
 	/* Accept only local observers */
 	if (from->sq_node != qrtr_ns.local_node)
 		return -EINVAL;
 
+	/* Make sure the client performs only maximum allowed lookups */
+	list_for_each_entry(lookup, &qrtr_ns.lookups, li) {
+		if (lookup->sq.sq_node == from->sq_node &&
+		    lookup->sq.sq_port == from->sq_port)
+			count++;
+	}
+
+	if (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;
-- 
2.51.0

Re: [PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node

[Simon Horman]

On Wed, Mar 25, 2026 at 04:14:14PM +0530, Manivannan Sadhasivam wrote:
> 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.
> 
> 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 <yimingqian591@gmail.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Reviewed-by: Simon Horman <horms@kernel.org>

> ---
>  net/qrtr/ns.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index 3203b2220860..fb4e8a2d370d 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;

This is not new behaviour added by patch, but If I understand things
correctly, node_get will allocate a new node if one doesn't already exist
for the node_id.

I am wondering if any bounds are placed on the number of nodes that can be
created. And, if not, is this a point of concern from a memory exhaustion
perspective?

...

Re: [PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket

[Simon Horman]

On Wed, Mar 25, 2026 at 04:14:15PM +0530, Manivannan Sadhasivam wrote:
> Current code does no bound checking on the number of lookups a client can
> perform per socket. 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 per socket.
> 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 <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  net/qrtr/ns.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index fb4e8a2d370d..707fde809939 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c
> @@ -70,10 +70,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)
>  {
> @@ -545,11 +546,24 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from,
>  	struct qrtr_node *node;
>  	unsigned long node_idx;
>  	unsigned long srv_idx;
> +	u8 count = 0;
>  
>  	/* Accept only local observers */
>  	if (from->sq_node != qrtr_ns.local_node)
>  		return -EINVAL;
>  
> +	/* Make sure the client performs only maximum allowed lookups */
> +	list_for_each_entry(lookup, &qrtr_ns.lookups, li) {
> +		if (lookup->sq.sq_node == from->sq_node &&
> +		    lookup->sq.sq_port == from->sq_port)
> +			count++;

This feels like it could get quite expensive.
If many lookups are added, it feels like it may be O(n^2).

Is this something that has been considered?

> +	}
> +
> +	if (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;
> -- 
> 2.51.0
> 

Re: [PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node

[Manivannan Sadhasivam]

On Fri, Mar 27, 2026 at 09:58:32AM +0000, Simon Horman wrote:
> On Wed, Mar 25, 2026 at 04:14:14PM +0530, Manivannan Sadhasivam wrote:
> > 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.
> > 
> > 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 <yimingqian591@gmail.com>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> > ---
> >  net/qrtr/ns.c | 24 ++++++++++++++++++++----
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> > index 3203b2220860..fb4e8a2d370d 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;
> 
> This is not new behaviour added by patch, but If I understand things
> correctly, node_get will allocate a new node if one doesn't already exist
> for the node_id.
> 

Yes!

> I am wondering if any bounds are placed on the number of nodes that can be
> created. And, if not, is this a point of concern from a memory exhaustion
> perspective?
> 

That's true. I plan to send a followup for that. This series just limits the
scope in addressing the reported issue.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket

[Manivannan Sadhasivam]

On Fri, Mar 27, 2026 at 10:07:09AM +0000, Simon Horman wrote:
> On Wed, Mar 25, 2026 at 04:14:15PM +0530, Manivannan Sadhasivam wrote:
> > Current code does no bound checking on the number of lookups a client can
> > perform per socket. 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 per socket.
> > 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 <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  net/qrtr/ns.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> > index fb4e8a2d370d..707fde809939 100644
> > --- a/net/qrtr/ns.c
> > +++ b/net/qrtr/ns.c
> > @@ -70,10 +70,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)
> >  {
> > @@ -545,11 +546,24 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from,
> >  	struct qrtr_node *node;
> >  	unsigned long node_idx;
> >  	unsigned long srv_idx;
> > +	u8 count = 0;
> >  
> >  	/* Accept only local observers */
> >  	if (from->sq_node != qrtr_ns.local_node)
> >  		return -EINVAL;
> >  
> > +	/* Make sure the client performs only maximum allowed lookups */
> > +	list_for_each_entry(lookup, &qrtr_ns.lookups, li) {
> > +		if (lookup->sq.sq_node == from->sq_node &&
> > +		    lookup->sq.sq_port == from->sq_port)
> > +			count++;
> 
> This feels like it could get quite expensive.
> If many lookups are added, it feels like it may be O(n^2).
> 

Lookups are not something that'll happen very often. A client only registers
for the lookup once per service that it depends on. That shouldn't be too
much. And then once lookup is registered, it will be used throughout the
lifetime of the client.

So there is no overhead associated with this check.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node

[Jakub Kicinski]

On Fri, 27 Mar 2026 15:40:01 +0530 Manivannan Sadhasivam wrote:
> > I am wondering if any bounds are placed on the number of nodes that can be
> > created. And, if not, is this a point of concern from a memory exhaustion
> > perspective?
> 
> That's true. I plan to send a followup for that. This series just limits the
> scope in addressing the reported issue.

This series is moot without such limit, tho.
Let's fix it all in one series.
I'll send you the remaining AI feedback.

Re: [PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: qrtr: ns: Limit the maximum server registration per node

This commit introduces a limit of 256 server registrations per node to
prevent malicious clients from exhausting memory by flooding NEW_SERVER
messages.

Cc: stable@vger.kernel.org
Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace")
Reported-by: Yiming Qian <yimingqian591@gmail.com>

> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index 3203b2220860..fb4e8a2d370d 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c

[ ... ]

> @@ -229,6 +235,17 @@ static struct qrtr_server *server_add(unsigned int service,
>  	if (!service || !port)
>  		return NULL;
>  
> +	node = node_get(node_id);

Does this per-node limit fully protect against memory exhaustion?

If a remote client spoofs the node ID in the NEW_SERVER message, the
attacker-controlled node_id is passed to server_add(). Because node_get()
unconditionally allocates a new qrtr_node structure when one is not found,
an attacker could continuously send messages with unique node IDs.

Could this result in unbounded node allocations that still exhaust system
memory despite the per-node server limit?

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

While this error message is rate-limited, returning NULL here causes
ctrl_cmd_new_server() to return -EINVAL.

In the main packet loop qrtr_ns_worker(), a negative return value triggers
an un-ratelimited error:

	if (ret < 0)
		pr_err("failed while handling packet from %d:%d",
		       sq.sq_node, sq.sq_port);

Could a flood of excess server registrations trigger this un-ratelimited
message and cause a console logging storm?

> +	}
> +
>  	srv = kzalloc_obj(*srv);
>  	if (!srv)
>  		return NULL;
-- 
pw-bot: cr

Re: [PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net: qrtr: ns: Limit the maximum lookups per socket

This commit limits the maximum number of lookups to 64 per socket to prevent
a malicious local client from sending a flood of NEW_LOOKUP messages over the
same socket.

> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index fb4e8a2d370d..707fde809939 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c

[ ... ]

> @@ -545,11 +546,24 @@ static int ctrl_cmd_new_lookup(struct sockaddr_qrtr *from,
>  	struct qrtr_node *node;
>  	unsigned long node_idx;
>  	unsigned long srv_idx;
> +	u8 count = 0;
>  
>  	/* Accept only local observers */
>  	if (from->sq_node != qrtr_ns.local_node)
>  		return -EINVAL;
>  
> +	/* Make sure the client performs only maximum allowed lookups */
> +	list_for_each_entry(lookup, &qrtr_ns.lookups, li) {
> +		if (lookup->sq.sq_node == from->sq_node &&
> +		    lookup->sq.sq_port == from->sq_port)
> +			count++;
> +	}

Does this O(N) traversal of the global qrtr_ns.lookups list create a CPU
exhaustion issue?

If a local attacker opens many sockets and registers the maximum 64 lookups
on each, the global list could grow to tens of thousands of items.

If the attacker then floods the socket with NEW_LOOKUP packets, the
single-threaded qrtr_ns.workqueue worker would be forced to traverse all
elements for every packet, potentially blocking all legitimate QRTR namespace
control traffic.

> +
> +	if (count >= QRTR_NS_MAX_LOOKUPS) {
> +		pr_err_ratelimited("QRTR client node exceeds max lookup limit!\n");
> +		return -ENOSPC;

Does returning -ENOSPC here defeat the ratelimiting?

If the caller qrtr_ns_worker() unconditionally checks for a negative return
value and executes an unratelimited error print, a malicious local client
could still cause a syslog flood by sending continuous NEW_LOOKUP packets
after hitting the limit.

Should this return 0 instead to avoid the unratelimited print in the caller?

> +	}
> +
>  	lookup = kzalloc_obj(*lookup);
>  	if (!lookup)
>  		return -ENOMEM;

This isn't a regression introduced by this patch, but does qrtr_ns_remove()
contain a use-after-free during module unload?

The teardown sequence appears to be:
1. cancel_work_sync()
2. destroy_workqueue(qrtr_ns.workqueue)
3. sock_release(qrtr_ns.sock)

If a packet arrives between destroy_workqueue() and sock_release(), the
socket's receive path triggers qrtr_ns_data_ready(). This callback calls
queue_work(qrtr_ns.workqueue, ...), which dereferences the already-freed
workqueue pointer.

Should the socket be shut down or the callback disabled before destroying
the workqueue?