All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, linux-arm-msm@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	andersson@kernel.org, yimingqian591@gmail.com,
	chris.lew@oss.qualcomm.com, mani@kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket
Date: Fri, 27 Mar 2026 10:07:09 +0000	[thread overview]
Message-ID: <20260327100709.GD111839@horms.kernel.org> (raw)
In-Reply-To: <20260325104415.104972-3-manivannan.sadhasivam@oss.qualcomm.com>

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
> 

  reply	other threads:[~2026-03-27 10:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-25 10:44 [PATCH 0/2] net: qrtr: ns: Fix unbounded server/lookup messages Manivannan Sadhasivam
2026-03-25 10:44 ` [PATCH 1/2] net: qrtr: ns: Limit the maximum server registration per node Manivannan Sadhasivam
2026-03-27  9:58   ` Simon Horman
2026-03-27 10:10     ` Manivannan Sadhasivam
2026-03-29 20:33       ` Jakub Kicinski
2026-04-03  8:30       ` Simon Horman
2026-04-03  8:52         ` Simon Horman
2026-03-29 20:33   ` Jakub Kicinski
2026-03-25 10:44 ` [PATCH 2/2] net: qrtr: ns: Limit the maximum lookups per socket Manivannan Sadhasivam
2026-03-27 10:07   ` Simon Horman [this message]
2026-03-27 10:17     ` Manivannan Sadhasivam
2026-04-03  8:32       ` Simon Horman
2026-03-29 20:33   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260327100709.GD111839@horms.kernel.org \
    --to=horms@kernel.org \
    --cc=andersson@kernel.org \
    --cc=chris.lew@oss.qualcomm.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mani@kernel.org \
    --cc=manivannan.sadhasivam@oss.qualcomm.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=yimingqian591@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.