Thread (1 message) 1 message, 1 author, 1d ago

Re: [PATCH net-next v15 05/15] quic: provide quic.h header files for kernel and userspace

From: netdev-bot+sashiko@kernel.org
Date: 2026-09-15 19:50:39
Also in: linux-cifs

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1

- [Low] Kernel-doc contradicts the exported interface contract: the new
  kernel-doc blocks added in net/quic/socket.c for quic_do_setsockopt()…

--

quic: provide quic.h header files for kernel and userspace

This patch adds include/uapi/linux/quic.h with the QUIC socket API
definitions (cmsgs, socket options, events) and adds include/linux/quic.h
so in-kernel consumers can reach quic_do_setsockopt()/quic_do_getsockopt()
now that kernel_get/setsockopt() is gone. The two functions lose their
static qualifier and are exported with EXPORT_SYMBOL_GPL().

Link: https://datatracker.ietf.org/doc/html/draft-lxin-quic-socket-apis
quoted hunk
diff --git a/include/linux/quic.h b/include/linux/quic.h
new file mode 100644
index 0000000000000..51c099e9547f5
--- /dev/null
+++ b/include/linux/quic.h
@@ -0,0 +1,38 @@
[ ... ]
+#include <linux/sockptr.h>
+#include <uapi/linux/quic.h>
+
+struct sock;
+
+#if IS_REACHABLE(CONFIG_IP_QUIC)
+int quic_do_setsockopt(struct sock *sk, int optname, sockptr_t optval,
+		       unsigned int optlen);
+int quic_do_getsockopt(struct sock *sk, int optname, sockptr_t optval,
+		       sockptr_t optlen);
[ ... ]
quoted hunk
diff --git a/net/quic/socket.c b/net/quic/socket.c
index 1c97d95c61b2c..0e0a13235fa42 100644
--- a/net/quic/socket.c
+++ b/net/quic/socket.c
@@ -111,11 +111,25 @@ static void quic_close(struct sock *sk, long timeout)
 	sk_common_release(sk);
 }
 
-static int quic_do_setsockopt(struct sock *sk, int optname, sockptr_t optval,
-			      unsigned int optlen)
+/**
+ * quic_do_setsockopt - set a QUIC socket option
+ * @sk: socket to configure
+ * @optname: option name (QUIC-level)
+ * @optval: user buffer containing the option value
                ^^^^^^^^^^^

[Severity: Low]
This isn't a functional bug, but should the kernel-doc for optval here say
"user buffer"?  The parameter is a sockptr_t, and the whole point of the new
include/linux/quic.h plus the EXPORT_SYMBOL_GPL() below is to let in-kernel
consumers call this with a kernel buffer:

include/linux/sockptr.h:
    /*
     * Support for "universal" pointers that can point to either kernel or
     * userspace memory.
     */
    ...
    static inline sockptr_t KERNEL_SOCKPTR(void *p)

Would something like "buffer (user or kernel, see sockptr_t) containing the
option value" describe the contract more accurately?
quoted hunk
+ * @optlen: size of the option value
+ *
+ * Sets a QUIC socket option on a given socket.
+ *
+ * Return:
+ * - On success, 0 is returned.
+ * - On error, a negative error value is returned.
+ */
+int quic_do_setsockopt(struct sock *sk, int optname, sockptr_t optval,
+		       unsigned int optlen)
 {
 	return -EOPNOTSUPP;
 }
+EXPORT_SYMBOL_GPL(quic_do_setsockopt);
 
 static int quic_setsockopt(struct sock *sk, int level, int optname,
 			   sockptr_t optval, unsigned int optlen)
@@ -127,11 +141,25 @@ static int quic_setsockopt(struct sock *sk, int level, int optname,
 	return quic_do_setsockopt(sk, optname, optval, optlen);
 }
 
-static int quic_do_getsockopt(struct sock *sk, int optname, sockptr_t optval,
-			      sockptr_t optlen)
+/**
+ * quic_do_getsockopt - get a QUIC socket option
+ * @sk: socket to query
+ * @optname: option name (QUIC-level)
+ * @optval: user buffer to receive the option value
+ * @optlen: pointer to buffer size; updated with actual size on return
                ^^^^^^^^^^^^^^^^^^^^

[Severity: Low]
Same question for quic_do_getsockopt(): optval is described as a "user
buffer" and optlen as a "pointer to buffer size", but both are sockptr_t
rather than plain or __user pointers.

The distinction is visible right below in quic_getsockopt(), which has to
wrap its own pointers:

    return quic_do_getsockopt(sk, optname, USER_SOCKPTR(optval),
                              USER_SOCKPTR(optlen));

while a kernel consumer would pass KERNEL_SOCKPTR().  Should the optlen
description mention it is a sockptr_t holding the buffer size, so callers
don't read this as a plain int pointer?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/cover.1789393775.git.lucien.xin%40gmail.com
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help