netdev - Re: [PATCH net-next v4 5/8] net: mctp: Use hashtable for binds

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <cd1d3ba89ef2c038b82968dff3c9c3de9843c0e2.camel@codeconstruct.com.au>
Date: Tue, 15 Jul 2025 19:08:03 +0800
From: Matt Johnston <matt@...econstruct.com.au>
To: Paolo Abeni <pabeni@...hat.com>, Jeremy Kerr <jk@...econstruct.com.au>, 
 "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Simon Horman
 <horms@...nel.org>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next v4 5/8] net: mctp: Use hashtable for binds

Hi Paolo,

Thanks for the review.

On Tue, 2025-07-15 at 12:05 +0200, Paolo Abeni wrote:
...
> >  	struct mutex bind_lock;
> > -	struct hlist_head binds;
> > +	DECLARE_HASHTABLE(binds, MCTP_BINDS_BITS);
> 
> Note that I comment on patch 2/8 before actually looking at this patch.

I guess even with a hash table there's theoretical scope for someone
to fill a hash bucket (via clever timing measurement perhaps). Currently MCTP
requires CAP_NET_BIND_SERVICE for any binds, so the risk there is limited.

> As a possible follow-up I suggest a dynamically allocating the hash
> table at first bind time.

*nod* I'll take a look at that.

> > ...
> > +	/* Look for binds in order of widening scope. A given destination or
> > +	 * source address also implies matching on a particular network.
> > +	 *
> > +	 * - Matching destination and source
> > +	 * - Matching destination
> > +	 * - Matching source
> > +	 * - Matching network, any address
> > +	 * - Any network or address
> > +	 */
> 
> Note for a possible follow-up: a more idiomatic approach uses a
> compute_score() function that respect the above priority and require a
> single hash traversal, see, i.e. net/ipv4/udp.c

Thanks for the pointer, I'll see if something like that would work.

Cheers,
Matt