netdev - Re: [PATCH net-next v2 07/15] quic: add connection id management

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1cf31726-bfb9-4909-a077-6c2c45e0720a@redhat.com>
Date: Thu, 21 Aug 2025 15:55:04 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Xin Long <lucien.xin@...il.com>, network dev <netdev@...r.kernel.org>
Cc: davem@...emloft.net, kuba@...nel.org, Eric Dumazet <edumazet@...gle.com>,
 Simon Horman <horms@...nel.org>, Stefan Metzmacher <metze@...ba.org>,
 Moritz Buhl <mbuhl@...nbsd.org>, Tyler Fanelli <tfanelli@...hat.com>,
 Pengtao He <hepengtao@...omi.com>, linux-cifs@...r.kernel.org,
 Steve French <smfrench@...il.com>, Namjae Jeon <linkinjeon@...nel.org>,
 Paulo Alcantara <pc@...guebit.com>, Tom Talpey <tom@...pey.com>,
 kernel-tls-handshake@...ts.linux.dev, Chuck Lever <chuck.lever@...cle.com>,
 Jeff Layton <jlayton@...nel.org>, Benjamin Coddington <bcodding@...hat.com>,
 Steve Dickson <steved@...hat.com>, Hannes Reinecke <hare@...e.de>,
 Alexander Aring <aahringo@...hat.com>, David Howells <dhowells@...hat.com>,
 Cong Wang <xiyou.wangcong@...il.com>, "D . Wythe"
 <alibuda@...ux.alibaba.com>, Jason Baron <jbaron@...mai.com>,
 illiliti <illiliti@...tonmail.com>, Sabrina Dubroca <sd@...asysnail.net>,
 Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
 Daniel Stenberg <daniel@...x.se>,
 Andy Gospodarek <andrew.gospodarek@...adcom.com>
Subject: Re: [PATCH net-next v2 07/15] quic: add connection id management

On 8/18/25 4:04 PM, Xin Long wrote:
> This patch introduces 'struct quic_conn_id_set' for managing Connection
> IDs (CIDs), which are represented by 'struct quic_source_conn_id'
> and 'struct quic_dest_conn_id'.
> 
> It provides helpers to add and remove CIDs from the set, and handles
> insertion of source CIDs into the global connection ID hash table
> when necessary.
> 
> - quic_conn_id_add(): Add a new Connection ID to the set, and inserts
>   it to conn_id hash table if it is a source conn_id.
> 
> - quic_conn_id_remove(): Remove connection IDs the set with sequence
>   numbers less than or equal to a number.

It's unclear how many connections are expected to be contained in each
set. If more than an handful you should consider using RB-tree instead
of lists.

[...]
> +static void quic_source_conn_id_free(struct quic_source_conn_id *s_conn_id)
> +{
> +	u8 *data = s_conn_id->common.id.data;
> +	struct quic_hash_head *head;
> +
> +	if (!hlist_unhashed(&s_conn_id->node)) {
> +		head = quic_source_conn_id_head(sock_net(s_conn_id->sk), data);
> +		spin_lock_bh(&head->s_lock);
> +		hlist_del_init(&s_conn_id->node);
> +		spin_unlock_bh(&head->s_lock);
> +	}
> +
> +	/* Freeing is deferred via RCU to avoid use-after-free during concurrent lookups. */
> +	call_rcu(&s_conn_id->rcu, quic_source_conn_id_free_rcu);
> +}
> +
> +static void quic_conn_id_del(struct quic_common_conn_id *common)
> +{
> +	list_del(&common->list);
> +	if (!common->hashed) {
> +		kfree(common);
> +		return;
> +	}
> +	quic_source_conn_id_free((struct quic_source_conn_id *)common);

It looks like the above cast is not needed.

> +}
> +
> +/* Add a connection ID with sequence number and associated private data to the connection ID set. */
> +int quic_conn_id_add(struct quic_conn_id_set *id_set,
> +		     struct quic_conn_id *conn_id, u32 number, void *data)
> +{
> +	struct quic_source_conn_id *s_conn_id;
> +	struct quic_dest_conn_id *d_conn_id;
> +	struct quic_common_conn_id *common;
> +	struct quic_hash_head *head;
> +	struct list_head *list;
> +
> +	/* Locate insertion point to keep list ordered by number. */
> +	list = &id_set->head;
> +	list_for_each_entry(common, list, list) {
> +		if (number == common->number)
> +			return 0; /* Ignore if it is already exists on the list. */
> +		if (number < common->number) {
> +			list = &common->list;
> +			break;
> +		}
> +	}
> +
> +	if (conn_id->len > QUIC_CONN_ID_MAX_LEN)
> +		return -EINVAL;
> +	common = kzalloc(id_set->entry_size, GFP_ATOMIC);
> +	if (!common)
> +		return -ENOMEM;
> +	common->id = *conn_id;
> +	common->number = number;
> +	if (id_set->entry_size == sizeof(struct quic_dest_conn_id)) {
> +		/* For destination connection IDs, copy the stateless reset token if available. */
> +		if (data) {
> +			d_conn_id = (struct quic_dest_conn_id *)common;
> +			memcpy(d_conn_id->token, data, QUIC_CONN_ID_TOKEN_LEN);
> +		}
> +	} else {
> +		/* For source connection IDs, mark as hashed and insert into the global source
> +		 * connection ID hashtable.
> +		 */
> +		common->hashed = 1;
> +		s_conn_id = (struct quic_source_conn_id *)common;
> +		s_conn_id->sk = data;
> +
> +		head = quic_source_conn_id_head(sock_net(s_conn_id->sk), common->id.data);
> +		spin_lock_bh(&head->s_lock);
> +		hlist_add_head(&s_conn_id->node, &head->head);
> +		spin_unlock_bh(&head->s_lock);
> +	}
> +	list_add_tail(&common->list, list);

It's unclear if/how id_set->list is protected vs concurrent accesses.

/P