Thread (30 messages) 30 messages, 3 authors, 2026-02-05

Re: [net-next,v9,03/15] quic: provide common utilities and data structures

From: Xin Long <lucien.xin@gmail.com>
Date: 2026-02-05 19:18:12
Also in: linux-cifs

On Thu, Feb 5, 2026 at 7:51 AM Paolo Abeni [off-list ref] wrote:
On 2/5/26 12:54 PM, Simon Horman wrote:
quoted
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
quic: provide common utilities and data structures

This patch provides foundational data structures and utilities for
the QUIC stack, including hash tables for socket lookup, connection
ID management, and helpers for encoding/decoding transport parameters.
quoted
diff --git a/net/quic/common.c b/net/quic/common.c
new file mode 100644
index 000000000000..94321f3f213d
--- /dev/null
+++ b/net/quic/common.c
[ ... ]
quoted
+/* Reads a QUIC transport parameter value. */
+u8 quic_get_param(u64 *pdest, u8 **pp, u32 *plen)
+{
+    u64 valuelen;
+
+    if (!quic_get_var(pp, plen, &valuelen))
+            return 0;
+
+    if (*plen < valuelen)
+            return 0;
+
+    if (!quic_get_var(pp, plen, pdest))
+            return 0;
+
+    return (u8)valuelen;
+}
Does quic_get_param() need to verify that quic_get_var() consumed exactly
valuelen bytes?

The function reads valuelen at the first quic_get_var() call, then decodes
the value with the second quic_get_var(). However, quic_get_var() determines
how many bytes to consume based on the varint prefix in the data, not from
valuelen.

If a malformed packet has valuelen=2 but the varint encoding requires 4 bytes,
quic_get_var() will consume 4 bytes while the function returns valuelen=2.
This could desynchronize parsing of subsequent parameters since the caller
expects the buffer to have advanced by valuelen bytes based on the return
value.
Hard to tell without any caller for this function. Possibly you could
drop it? IDK if enforcing such check would be correct/safe from an
interoperability PoV.
I kind of forgot why I didn't validate it, I will decide if it should be
added after interop testing with those userland implementations.
quoted
quoted
diff --git a/net/quic/common.h b/net/quic/common.h
new file mode 100644
index 000000000000..bfec0aaf2907
--- /dev/null
+++ b/net/quic/common.h
[ ... ]
quoted
+#define QUIC_CONN_ID_MAX_LEN        20
[ ... ]
quoted
+struct quic_conn_id {
+    u8 data[QUIC_CONN_ID_MAX_LEN];
+    u8 len;
+};
+
+static inline void quic_conn_id_update(struct quic_conn_id *conn_id, u8 *data, u32 len)
+{
+    memcpy(conn_id->data, data, len);
+    conn_id->len = (u8)len;
+}
Should quic_conn_id_update() validate that len does not exceed
QUIC_CONN_ID_MAX_LEN before the memcpy?

The destination buffer conn_id->data is a 20-byte array, but the len
parameter is u32 with no bounds check. If a caller passes len > 20, the
memcpy would overflow the buffer.
It looks like the check responsibility is on the callers. I guess it
would make things more robust and simpler move the check here.
Parsing the connid from incoming packets will always check its length,
not good to move the check here then return an error for coding.

I will only leave a comment here. like:

/* Caller must ensure len does not exceed QUIC_CONN_ID_MAX_LEN. */

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help