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

[Simon Horman]

On Fri, Apr 03, 2026 at 09:30:09AM +0100, Simon Horman wrote:
> On Fri, Mar 27, 2026 at 03:40:01PM +0530, Manivannan Sadhasivam wrote:
> > 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@xxxxxxxxxxxxxxx
> > > > Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace")
> > > > Reported-by: Yiming Qian <yimingqian591@xxxxxxxxx>
> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > > 
> > > Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
> > > 
> > > > ---
> > > >  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.
> 
> Thanks, sounds good to me.
> 
> For this patch, feel free to add:
> 
> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>

Sorry, I now see Jakub's comment on this.
So I'll review the revised series, addressing the concern above,
when it becomes available.

Feel free to include my Reviewed-by tags on patches that
don't change in a material way.