[PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands

[PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands

[Breno Leitao]

This patchset adds support for getsockopt (SOCKET_URING_OP_GETSOCKOPT)
and setsockopt (SOCKET_URING_OP_SETSOCKOPT) in io_uring commands.
SOCKET_URING_OP_SETSOCKOPT implements generic case, covering all levels
nad optnames. On the other hand, SOCKET_URING_OP_GETSOCKOPT just
implements level SOL_SOCKET case, which seems to be the
most common level parameter for get/setsockopt(2).

struct proto_ops->setsockopt() uses sockptr instead of userspace
pointers, which makes it easy to bind to io_uring. Unfortunately
proto_ops->getsockopt() callback uses userspace pointers, except for
SOL_SOCKET, which is handled by sk_getsockopt(). Thus, this patchset
leverages sk_getsockopt() to imlpement the SOCKET_URING_OP_GETSOCKOPT
case.

In order to support BPF hooks, I modified the hooks to use  sockptr, so,
it is flexible enough to accept user or kernel pointers for
optval/optlen.

PS1: For getsockopt command, the optlen field is not a userspace
pointers, but an absolute value, so this is slightly different from
getsockopt(2) behaviour. The new optlen value is returned in cqe->res.

PS2: The userspace pointers need to be alive until the operation is
completed.

These changes were tested with a new test[1] in liburing. On the BPF
side, I tested that no regression was introduced by running "test_progs"
self test using "sockopt" test case.

[1] Link: https://github.com/leitao/liburing/blob/getsock/test/socket-getsetsock-cmd.c

RFC -> V1:
	* Copy user memory at io_uring subsystem, and call proto_ops
	  callbacks using kernel memory
	* Implement all the cases for SOCKET_URING_OP_SETSOCKOPT
V1 -> V2
	* Implemented the BPF part
	* Using user pointers from optval to avoid kmalloc in io_uring part.

Breno Leitao (8):
  net: expose sock_use_custom_sol_socket
  io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT
  io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT
  io_uring/cmd: Extend support beyond SOL_SOCKET
  bpf: Leverage sockptr_t in BPF getsockopt hook
  bpf: Leverage sockptr_t in BPF setsockopt hook
  io_uring/cmd: BPF hook for getsockopt cmd
  io_uring/cmd: BPF hook for setsockopt cmd

 include/linux/bpf-cgroup.h    |  7 +--
 include/linux/net.h           |  5 +++
 include/uapi/linux/io_uring.h |  8 ++++
 io_uring/uring_cmd.c          | 82 +++++++++++++++++++++++++++++++++++
 kernel/bpf/cgroup.c           | 25 ++++++-----
 net/socket.c                  | 12 ++---
 6 files changed, 117 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Breno Leitao]

Exposing function sock_use_custom_sol_socket(), so it could be used by
io_uring subsystem.

This function will be used in the function io_uring_cmd_setsockopt() in
the coming patch, so, let's move it to the socket.h header file.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/net.h | 5 +++++
 net/socket.c        | 5 -----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index 41c608c1b02c..14a956e4530e 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -355,4 +355,9 @@ u32 kernel_sock_ip_overhead(struct sock *sk);
 #define MODULE_ALIAS_NET_PF_PROTO_NAME(pf, proto, name) \
 	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
 		     name)
+
+static inline bool sock_use_custom_sol_socket(const struct socket *sock)
+{
+	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
+}
 #endif	/* _LINUX_NET_H */
diff --git a/net/socket.c b/net/socket.c
index 1dc23f5298ba..8df54352af83 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2216,11 +2216,6 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
 	return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
 }
 
-static bool sock_use_custom_sol_socket(const struct socket *sock)
-{
-	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
-}
-
 /*
  *	Set a socket option. Because we don't know the option lengths we have
  *	to pass the user mode parameter for the protocols to sort out.
-- 
2.34.1

[PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT

[Breno Leitao]

Add support for getsockopt command (SOCKET_URING_OP_GETSOCKOPT), where
level is SOL_SOCKET. This is leveraging the sockptr_t infrastructure,
where a sockptr_t is either userspace or kernel space, and handled as
such.

Function io_uring_cmd_getsockopt() is inspired by __sys_getsockopt().

Differently from the getsockopt(2), the optlen field is not a userspace
pointers. In getsockopt(2), userspace provides optlen pointer, which is
overwritten by the kernel.  In this implementation, userspace passes a
u32, and the new value is returned in cqe->res. I.e., optlen is not a
pointer.

Important to say that userspace needs to keep the pointer alive until
the CQE is completed.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/uapi/linux/io_uring.h |  7 +++++++
 io_uring/uring_cmd.c          | 28 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 9fc7195f25df..8152151080db 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -43,6 +43,10 @@ struct io_uring_sqe {
 	union {
 		__u64	addr;	/* pointer to buffer or iovecs */
 		__u64	splice_off_in;
+		struct {
+			__u32	level;
+			__u32	optname;
+		};
 	};
 	__u32	len;		/* buffer size or number of iovecs */
 	union {
@@ -79,6 +83,7 @@ struct io_uring_sqe {
 	union {
 		__s32	splice_fd_in;
 		__u32	file_index;
+		__u32	optlen;
 		struct {
 			__u16	addr_len;
 			__u16	__pad3[1];
@@ -89,6 +94,7 @@ struct io_uring_sqe {
 			__u64	addr3;
 			__u64	__pad2[1];
 		};
+		__u64	optval;
 		/*
 		 * If the ring is initialized with IORING_SETUP_SQE128, then
 		 * this field is used for 80 bytes of arbitrary command data
@@ -729,6 +735,7 @@ struct io_uring_recvmsg_out {
 enum {
 	SOCKET_URING_OP_SIOCINQ		= 0,
 	SOCKET_URING_OP_SIOCOUTQ,
+	SOCKET_URING_OP_GETSOCKOPT,
 };
 
 #ifdef __cplusplus
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 8e7a03c1b20e..582931879482 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -166,6 +166,32 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
 }
 EXPORT_SYMBOL_GPL(io_uring_cmd_import_fixed);
 
+static inline int io_uring_cmd_getsockopt(struct socket *sock,
+					  struct io_uring_cmd *cmd)
+{
+	void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
+	int optname = READ_ONCE(cmd->sqe->optname);
+	int optlen = READ_ONCE(cmd->sqe->optlen);
+	int level = READ_ONCE(cmd->sqe->level);
+	int err;
+
+	err = security_socket_getsockopt(sock, level, optname);
+	if (err)
+		return err;
+
+	if (level == SOL_SOCKET) {
+		err = sk_getsockopt(sock->sk, level, optname,
+				    USER_SOCKPTR(optval),
+				    KERNEL_SOCKPTR(&optlen));
+		if (err)
+			return err;
+
+		return optlen;
+	}
+
+	return -EOPNOTSUPP;
+}
+
 int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	struct socket *sock = cmd->file->private_data;
@@ -187,6 +213,8 @@ int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
 		if (ret)
 			return ret;
 		return arg;
+	case SOCKET_URING_OP_GETSOCKOPT:
+		return io_uring_cmd_getsockopt(sock, cmd);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.34.1

[PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT

[Breno Leitao]

Add initial support for SOCKET_URING_OP_SETSOCKOPT. This new command is
similar to setsockopt. This initial implementation just cares about
SOL_SOCKET level for now. The next patch implements the generic case.

Function io_uring_cmd_setsockopt() is inspired by the function
__sys_setsockopt().

Important to say that userspace needs to keep the pointer's memory alive
until the operation is completed.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/uapi/linux/io_uring.h |  1 +
 io_uring/uring_cmd.c          | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
index 8152151080db..3fe82df06abf 100644
--- a/include/uapi/linux/io_uring.h
+++ b/include/uapi/linux/io_uring.h
@@ -736,6 +736,7 @@ enum {
 	SOCKET_URING_OP_SIOCINQ		= 0,
 	SOCKET_URING_OP_SIOCOUTQ,
 	SOCKET_URING_OP_GETSOCKOPT,
+	SOCKET_URING_OP_SETSOCKOPT,
 };
 
 #ifdef __cplusplus
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 582931879482..5404b788ca14 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -192,6 +192,27 @@ static inline int io_uring_cmd_getsockopt(struct socket *sock,
 	return -EOPNOTSUPP;
 }
 
+static inline int io_uring_cmd_setsockopt(struct socket *sock,
+					  struct io_uring_cmd *cmd)
+{
+	void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
+	int optname = READ_ONCE(cmd->sqe->optname);
+	int optlen = READ_ONCE(cmd->sqe->optlen);
+	int level = READ_ONCE(cmd->sqe->level);
+	int err;
+
+	err = security_socket_setsockopt(sock, level, optname);
+	if (err)
+		return err;
+
+	err = -EOPNOTSUPP;
+	if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
+		err = sock_setsockopt(sock, level, optname,
+				      USER_SOCKPTR(optval), optlen);
+
+	return err;
+}
+
 int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
 {
 	struct socket *sock = cmd->file->private_data;
@@ -215,6 +236,8 @@ int io_uring_cmd_sock(struct io_uring_cmd *cmd, unsigned int issue_flags)
 		return arg;
 	case SOCKET_URING_OP_GETSOCKOPT:
 		return io_uring_cmd_getsockopt(sock, cmd);
+	case SOCKET_URING_OP_SETSOCKOPT:
+		return io_uring_cmd_setsockopt(sock, cmd);
 	default:
 		return -EOPNOTSUPP;
 	}
-- 
2.34.1

[PATCH v2 4/8] io_uring/cmd: Extend support beyond SOL_SOCKET

[Breno Leitao]

Add generic support for SOCKET_URING_OP_SETSOCKOPT, expanding the
current case, that just execute if level is SOL_SOCKET.

This implementation basically calls sock->ops->setsockopt() with a
kernel allocated optval;

Since the callback for ops->setsockopt() is already using sockptr_t,
then the callbacks are leveraged to be called directly, similarly to
__sys_setsockopt().

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 io_uring/uring_cmd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 5404b788ca14..dbba005a7290 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -205,10 +205,14 @@ static inline int io_uring_cmd_setsockopt(struct socket *sock,
 	if (err)
 		return err;
 
-	err = -EOPNOTSUPP;
 	if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
 		err = sock_setsockopt(sock, level, optname,
 				      USER_SOCKPTR(optval), optlen);
+	else if (unlikely(!sock->ops->setsockopt))
+		err = -EOPNOTSUPP;
+	else
+		err = sock->ops->setsockopt(sock, level, optname,
+					    USER_SOCKPTR(koptval), optlen);
 
 	return err;
 }
-- 
2.34.1

[PATCH v2 5/8] bpf: Leverage sockptr_t in BPF getsockopt hook

[Breno Leitao]

Leverage sockptr_t structure to have an argument that is either an
userspace pointer, or, a kernel pointer.

This makes this function flexible, so, we can mix and match user and
kernel space pointers. The main motivation for this change is to use it
in the io_uring {g,s}etsockopt(), which will use a userspace pointer for
*optval, but, a kernel value for optlen.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/bpf-cgroup.h |  5 +++--
 kernel/bpf/cgroup.c        | 20 +++++++++++---------
 net/socket.c               |  5 +++--
 3 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 57e9e109257e..d16cb99fd4f1 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -139,9 +139,10 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
 int __cgroup_bpf_run_filter_setsockopt(struct sock *sock, int *level,
 				       int *optname, char __user *optval,
 				       int *optlen, char **kernel_optval);
+
 int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
-				       int optname, char __user *optval,
-				       int __user *optlen, int max_optlen,
+				       int optname, sockptr_t optval,
+				       sockptr_t optlen, int max_optlen,
 				       int retval);
 
 int __cgroup_bpf_run_filter_getsockopt_kern(struct sock *sk, int level,
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 5b2741aa0d9b..ebc8c58f7e46 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1875,8 +1875,8 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
 }
 
 int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
-				       int optname, char __user *optval,
-				       int __user *optlen, int max_optlen,
+				       int optname, sockptr_t optval,
+				       sockptr_t optlen, int max_optlen,
 				       int retval)
 {
 	struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
@@ -1903,8 +1903,8 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
 		 * one that kernel returned as well to let
 		 * BPF programs inspect the value.
 		 */
-
-		if (get_user(ctx.optlen, optlen)) {
+		if (copy_from_sockptr(&ctx.optlen, optlen,
+				      sizeof(ctx.optlen))) {
 			ret = -EFAULT;
 			goto out;
 		}
@@ -1915,8 +1915,8 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
 		}
 		orig_optlen = ctx.optlen;
 
-		if (copy_from_user(ctx.optval, optval,
-				   min(ctx.optlen, max_optlen)) != 0) {
+		if (copy_from_sockptr(ctx.optval, optval,
+				      min(ctx.optlen, max_optlen))) {
 			ret = -EFAULT;
 			goto out;
 		}
@@ -1930,7 +1930,8 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
 	if (ret < 0)
 		goto out;
 
-	if (optval && (ctx.optlen > max_optlen || ctx.optlen < 0)) {
+	if (!sockptr_is_null(optval) &&
+	    (ctx.optlen > max_optlen || ctx.optlen < 0)) {
 		if (orig_optlen > PAGE_SIZE && ctx.optlen >= 0) {
 			pr_info_once("bpf getsockopt: ignoring program buffer with optlen=%d (max_optlen=%d)\n",
 				     ctx.optlen, max_optlen);
@@ -1942,11 +1943,12 @@ int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
 	}
 
 	if (ctx.optlen != 0) {
-		if (optval && copy_to_user(optval, ctx.optval, ctx.optlen)) {
+		if (!sockptr_is_null(optval) &&
+		    copy_to_sockptr(optval, ctx.optval, ctx.optlen)) {
 			ret = -EFAULT;
 			goto out;
 		}
-		if (put_user(ctx.optlen, optlen)) {
+		if (copy_to_sockptr(optlen, &ctx.optlen, sizeof(ctx.optlen))) {
 			ret = -EFAULT;
 			goto out;
 		}
diff --git a/net/socket.c b/net/socket.c
index 8df54352af83..c686c6e89441 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2306,8 +2306,9 @@ int __sys_getsockopt(int fd, int level, int optname, char __user *optval,
 
 	if (!in_compat_syscall())
 		err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level, optname,
-						     optval, optlen, max_optlen,
-						     err);
+						     USER_SOCKPTR(optval),
+						     USER_SOCKPTR(optlen),
+						     max_optlen, err);
 out_put:
 	fput_light(sock->file, fput_needed);
 	return err;
-- 
2.34.1

[PATCH v2 6/8] bpf: Leverage sockptr_t in BPF setsockopt hook

[Breno Leitao]

Move BPF setsockopt hook (__cgroup_bpf_run_filter_setsockopt()) to use
sockptr instead of user pointers. This brings flexibility to the
function, since it could be called with userspace or kernel pointers.

This also aligns with the getsockopt() counterpart, which is now using
sockptr_t types.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 include/linux/bpf-cgroup.h | 2 +-
 kernel/bpf/cgroup.c        | 5 +++--
 net/socket.c               | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index d16cb99fd4f1..5e3419eb267a 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -137,7 +137,7 @@ int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
 				   enum cgroup_bpf_attach_type atype);
 
 int __cgroup_bpf_run_filter_setsockopt(struct sock *sock, int *level,
-				       int *optname, char __user *optval,
+				       int *optname, sockptr_t optval,
 				       int *optlen, char **kernel_optval);
 
 int __cgroup_bpf_run_filter_getsockopt(struct sock *sk, int level,
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index ebc8c58f7e46..f0dedd4f7f2e 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1785,7 +1785,7 @@ static bool sockopt_buf_allocated(struct bpf_sockopt_kern *ctx,
 }
 
 int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
-				       int *optname, char __user *optval,
+				       int *optname, sockptr_t optval,
 				       int *optlen, char **kernel_optval)
 {
 	struct cgroup *cgrp = sock_cgroup_ptr(&sk->sk_cgrp_data);
@@ -1808,7 +1808,8 @@ int __cgroup_bpf_run_filter_setsockopt(struct sock *sk, int *level,
 
 	ctx.optlen = *optlen;
 
-	if (copy_from_user(ctx.optval, optval, min(*optlen, max_optlen)) != 0) {
+	if (copy_from_sockptr(ctx.optval, optval,
+			      min(*optlen, max_optlen))) {
 		ret = -EFAULT;
 		goto out;
 	}
diff --git a/net/socket.c b/net/socket.c
index c686c6e89441..b7d22633995a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2241,7 +2241,7 @@ int __sys_setsockopt(int fd, int level, int optname, char __user *user_optval,
 
 	if (!in_compat_syscall())
 		err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, &level, &optname,
-						     user_optval, &optlen,
+						     optval, &optlen,
 						     &kernel_optval);
 	if (err < 0)
 		goto out_put;
-- 
2.34.1

[PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd

[Breno Leitao]

Add BPF hooks support for getsockopts io_uring command. So, bpf cgroups
programs can run when SOCKET_URING_OP_GETSOCKOPT command is called.

This implementation follows a similar approach to what
__sys_getsockopt() does, but, using USER_SOCKPTR() for optval instead of
kernel pointer.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 io_uring/uring_cmd.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index dbba005a7290..3693e5779229 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -5,6 +5,8 @@
 #include <linux/io_uring.h>
 #include <linux/security.h>
 #include <linux/nospec.h>
+#include <linux/compat.h>
+#include <linux/bpf-cgroup.h>
 
 #include <uapi/linux/io_uring.h>
 #include <uapi/asm-generic/ioctls.h>
@@ -179,17 +181,23 @@ static inline int io_uring_cmd_getsockopt(struct socket *sock,
 	if (err)
 		return err;
 
-	if (level == SOL_SOCKET) {
+	err = -EOPNOTSUPP;
+	if (level == SOL_SOCKET)
 		err = sk_getsockopt(sock->sk, level, optname,
 				    USER_SOCKPTR(optval),
 				    KERNEL_SOCKPTR(&optlen));
-		if (err)
-			return err;
 
+	if (!in_compat_syscall())
+		err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
+						     optname,
+						     USER_SOCKPTR(optval),
+						     KERNEL_SOCKPTR(&optlen),
+						     optlen, err);
+
+	if (!err)
 		return optlen;
-	}
 
-	return -EOPNOTSUPP;
+	return err;
 }
 
 static inline int io_uring_cmd_setsockopt(struct socket *sock,
-- 
2.34.1

[PATCH v2 8/8] io_uring/cmd: BPF hook for setsockopt cmd

[Breno Leitao]

Add support for BPF hooks for io_uring setsockopts command.

This implementation follows a similar approach to what
__sys_setsockopt() does, but, operates only on kernel memory instead of
user memory (which is also possible, but not preferred since the kernel
memory is already available)

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 io_uring/uring_cmd.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 3693e5779229..b7b27e4dbddd 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -205,23 +205,42 @@ static inline int io_uring_cmd_setsockopt(struct socket *sock,
 {
 	void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
 	int optname = READ_ONCE(cmd->sqe->optname);
+	sockptr_t optval_s = USER_SOCKPTR(optval);
 	int optlen = READ_ONCE(cmd->sqe->optlen);
 	int level = READ_ONCE(cmd->sqe->level);
+	char *kernel_optval = NULL;
 	int err;
 
 	err = security_socket_setsockopt(sock, level, optname);
 	if (err)
 		return err;
 
+	if (!in_compat_syscall()) {
+		err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, &level,
+						     &optname,
+						     USER_SOCKPTR(optval),
+						     &optlen,
+						     &kernel_optval);
+		if (err < 0)
+			return err;
+		if (err > 0)
+			return 0;
+
+		/* Replace optval by the one returned by BPF */
+		if (kernel_optval)
+			optval_s = KERNEL_SOCKPTR(kernel_optval);
+	}
+
 	if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
 		err = sock_setsockopt(sock, level, optname,
-				      USER_SOCKPTR(optval), optlen);
+				      optval_s, optlen);
 	else if (unlikely(!sock->ops->setsockopt))
 		err = -EOPNOTSUPP;
 	else
 		err = sock->ops->setsockopt(sock, level, optname,
-					    USER_SOCKPTR(koptval), optlen);
+					    optval_s, optlen);
 
+	kfree(kernel_optval);
 	return err;
 }
 
-- 
2.34.1

Re: [PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Hugo Villeneuve]

On Tue,  8 Aug 2023 06:40:41 -0700
Breno Leitao <leitao@debian.org> wrote:

> Exposing function sock_use_custom_sol_socket(), so it could be used by
> io_uring subsystem.
> 
> This function will be used in the function io_uring_cmd_setsockopt() in
> the coming patch, so, let's move it to the socket.h header file.

Hi,
this description doesn't seem to match the code change below...

Hugo Villeneuve.


> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  include/linux/net.h | 5 +++++
>  net/socket.c        | 5 -----
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/net.h b/include/linux/net.h
> index 41c608c1b02c..14a956e4530e 100644
> --- a/include/linux/net.h
> +++ b/include/linux/net.h
> @@ -355,4 +355,9 @@ u32 kernel_sock_ip_overhead(struct sock *sk);
>  #define MODULE_ALIAS_NET_PF_PROTO_NAME(pf, proto, name) \
>  	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
>  		     name)
> +
> +static inline bool sock_use_custom_sol_socket(const struct socket *sock)
> +{
> +	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> +}
>  #endif	/* _LINUX_NET_H */
> diff --git a/net/socket.c b/net/socket.c
> index 1dc23f5298ba..8df54352af83 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2216,11 +2216,6 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
>  	return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
>  }
>  
> -static bool sock_use_custom_sol_socket(const struct socket *sock)
> -{
> -	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> -}
> -
>  /*
>   *	Set a socket option. Because we don't know the option lengths we have
>   *	to pass the user mode parameter for the protocols to sort out.
> -- 
> 2.34.1
> 

Re: [PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Breno Leitao]

Hello  Hugo,

On Tue, Aug 08, 2023 at 12:13:23PM -0400, Hugo Villeneuve wrote:
> On Tue,  8 Aug 2023 06:40:41 -0700
> Breno Leitao <leitao@debian.org> wrote:
> 
> > Exposing function sock_use_custom_sol_socket(), so it could be used by
> > io_uring subsystem.
> > 
> > This function will be used in the function io_uring_cmd_setsockopt() in
> > the coming patch, so, let's move it to the socket.h header file.
> 
> Hi,
> this description doesn't seem to match the code change below...

I re-read the patch comment and it seems to match what the code does,
so, probably this description only makes sense to me (?).

That said, hat have you understood from reading the description above?

Thanks for the review,

> > ---
> >  include/linux/net.h | 5 +++++
> >  net/socket.c        | 5 -----
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/include/linux/net.h b/include/linux/net.h
> > index 41c608c1b02c..14a956e4530e 100644
> > --- a/include/linux/net.h
> > +++ b/include/linux/net.h
> > @@ -355,4 +355,9 @@ u32 kernel_sock_ip_overhead(struct sock *sk);
> >  #define MODULE_ALIAS_NET_PF_PROTO_NAME(pf, proto, name) \
> >  	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
> >  		     name)
> > +
> > +static inline bool sock_use_custom_sol_socket(const struct socket *sock)
> > +{
> > +	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> > +}
> >  #endif	/* _LINUX_NET_H */
> > diff --git a/net/socket.c b/net/socket.c
> > index 1dc23f5298ba..8df54352af83 100644
> > --- a/net/socket.c
> > +++ b/net/socket.c
> > @@ -2216,11 +2216,6 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
> >  	return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
> >  }
> >  
> > -static bool sock_use_custom_sol_socket(const struct socket *sock)
> > -{
> > -	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> > -}
> > -
> >  /*
> >   *	Set a socket option. Because we don't know the option lengths we have
> >   *	to pass the user mode parameter for the protocols to sort out.
> > -- 
> > 2.34.1
> > 

Re: [PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands

[Stanislav Fomichev]

On 08/08, Breno Leitao wrote:
> This patchset adds support for getsockopt (SOCKET_URING_OP_GETSOCKOPT)
> and setsockopt (SOCKET_URING_OP_SETSOCKOPT) in io_uring commands.
> SOCKET_URING_OP_SETSOCKOPT implements generic case, covering all levels
> nad optnames. On the other hand, SOCKET_URING_OP_GETSOCKOPT just
> implements level SOL_SOCKET case, which seems to be the
> most common level parameter for get/setsockopt(2).
> 
> struct proto_ops->setsockopt() uses sockptr instead of userspace
> pointers, which makes it easy to bind to io_uring. Unfortunately
> proto_ops->getsockopt() callback uses userspace pointers, except for
> SOL_SOCKET, which is handled by sk_getsockopt(). Thus, this patchset
> leverages sk_getsockopt() to imlpement the SOCKET_URING_OP_GETSOCKOPT
> case.
> 
> In order to support BPF hooks, I modified the hooks to use  sockptr, so,
> it is flexible enough to accept user or kernel pointers for
> optval/optlen.
> 
> PS1: For getsockopt command, the optlen field is not a userspace
> pointers, but an absolute value, so this is slightly different from
> getsockopt(2) behaviour. The new optlen value is returned in cqe->res.
> 
> PS2: The userspace pointers need to be alive until the operation is
> completed.
> 
> These changes were tested with a new test[1] in liburing. On the BPF
> side, I tested that no regression was introduced by running "test_progs"
> self test using "sockopt" test case.
> 
> [1] Link: https://github.com/leitao/liburing/blob/getsock/test/socket-getsetsock-cmd.c
> 
> RFC -> V1:
> 	* Copy user memory at io_uring subsystem, and call proto_ops
> 	  callbacks using kernel memory
> 	* Implement all the cases for SOCKET_URING_OP_SETSOCKOPT

I did a quick pass, will take a close look later today. So far everything makes
sense to me.

Should we properly test it as well?
We have tools/testing/selftests/bpf/prog_tests/sockopt.c which does
most of the sanity checks, but it uses regular socket/{g,s}etsockopt
syscalls. Seems like it should be pretty easy to extend this with
io_uring path? tools/testing/selftests/net/io_uring_zerocopy_tx.c
already implements minimal wrappers which we can most likely borrow.

Re: [PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Hugo Villeneuve]

On Tue, 8 Aug 2023 10:21:03 -0700
Breno Leitao <leitao@debian.org> wrote:

> Hello  Hugo,
> 
> On Tue, Aug 08, 2023 at 12:13:23PM -0400, Hugo Villeneuve wrote:
> > On Tue,  8 Aug 2023 06:40:41 -0700
> > Breno Leitao <leitao@debian.org> wrote:
> > 
> > > Exposing function sock_use_custom_sol_socket(), so it could be used by
> > > io_uring subsystem.
> > > 
> > > This function will be used in the function io_uring_cmd_setsockopt() in
> > > the coming patch, so, let's move it to the socket.h header file.
> > 
> > Hi,
> > this description doesn't seem to match the code change below...
> 
> I re-read the patch comment and it seems to match what the code does,
> so, probably this description only makes sense to me (?).
> 
> That said, hat have you understood from reading the description above?
> socket.h
> Thanks for the review,

Hi Breno,
your comments says "move it to the socket.h header file" but it seems
to be moved to the net.h header file?

Hugo Villeneuve


> > > ---
> > >  include/linux/net.h | 5 +++++
> > >  net/socket.c        | 5 -----
> > >  2 files changed, 5 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/include/linux/net.h b/include/linux/net.h
> > > index 41c608c1b02c..14a956e4530e 100644
> > > --- a/include/linux/net.h
> > > +++ b/include/linux/net.h
> > > @@ -355,4 +355,9 @@ u32 kernel_sock_ip_overhead(struct sock *sk);
> > >  #define MODULE_ALIAS_NET_PF_PROTO_NAME(pf, proto, name) \
> > >  	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
> > >  		     name)
> > > +
> > > +static inline bool sock_use_custom_sol_socket(const struct socket *sock)
> > > +{
> > > +	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> > > +}
> > >  #endif	/* _LINUX_NET_H */
> > > diff --git a/net/socket.c b/net/socket.c
> > > index 1dc23f5298ba..8df54352af83 100644
> > > --- a/net/socket.c
> > > +++ b/net/socket.c
> > > @@ -2216,11 +2216,6 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
> > >  	return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
> > >  }
> > >  
> > > -static bool sock_use_custom_sol_socket(const struct socket *sock)
> > > -{
> > > -	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
> > > -}
> > > -
> > >  /*
> > >   *	Set a socket option. Because we don't know the option lengths we have
> > >   *	to pass the user mode parameter for the protocols to sort out.
> > > -- 
> > > 2.34.1
> > > 

Re: [PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Jeff Moyer]

Breno Leitao <leitao@debian.org> writes:

> Hello  Hugo,
>
> On Tue, Aug 08, 2023 at 12:13:23PM -0400, Hugo Villeneuve wrote:
>> On Tue,  8 Aug 2023 06:40:41 -0700
>> Breno Leitao <leitao@debian.org> wrote:
>> 
>> > Exposing function sock_use_custom_sol_socket(), so it could be used by
>> > io_uring subsystem.
>> > 
>> > This function will be used in the function io_uring_cmd_setsockopt() in
>> > the coming patch, so, let's move it to the socket.h header file.
>> 
>> Hi,
>> this description doesn't seem to match the code change below...
>
> I re-read the patch comment and it seems to match what the code does,
> so, probably this description only makes sense to me (?).
>
> That said, hat have you understood from reading the description above?

The comment states the function prototype is moving to socket.h, but the
patch puts it in net.h.

Cheers,
Jeff

>
> Thanks for the review,
>
>> > ---
>> >  include/linux/net.h | 5 +++++
>> >  net/socket.c        | 5 -----
>> >  2 files changed, 5 insertions(+), 5 deletions(-)
>> > 
>> > diff --git a/include/linux/net.h b/include/linux/net.h
>> > index 41c608c1b02c..14a956e4530e 100644
>> > --- a/include/linux/net.h
>> > +++ b/include/linux/net.h
>> > @@ -355,4 +355,9 @@ u32 kernel_sock_ip_overhead(struct sock *sk);
>> >  #define MODULE_ALIAS_NET_PF_PROTO_NAME(pf, proto, name) \
>> >  	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
>> >  		     name)
>> > +
>> > +static inline bool sock_use_custom_sol_socket(const struct socket *sock)
>> > +{
>> > +	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
>> > +}
>> >  #endif	/* _LINUX_NET_H */
>> > diff --git a/net/socket.c b/net/socket.c
>> > index 1dc23f5298ba..8df54352af83 100644
>> > --- a/net/socket.c
>> > +++ b/net/socket.c
>> > @@ -2216,11 +2216,6 @@ SYSCALL_DEFINE4(recv, int, fd, void __user *, ubuf, size_t, size,
>> >  	return __sys_recvfrom(fd, ubuf, size, flags, NULL, NULL);
>> >  }
>> >  
>> > -static bool sock_use_custom_sol_socket(const struct socket *sock)
>> > -{
>> > -	return test_bit(SOCK_CUSTOM_SOCKOPT, &sock->flags);
>> > -}
>> > -
>> >  /*
>> >   *	Set a socket option. Because we don't know the option lengths we have
>> >   *	to pass the user mode parameter for the protocols to sort out.
>> > -- 
>> > 2.34.1
>> > 

Re: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20230808]
[cannot apply to bpf-next/master bpf/master net/main net-next/main linus/master horms-ipvs/master v6.5-rc5 v6.5-rc4 v6.5-rc3 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/net-expose-sock_use_custom_sol_socket/20230809-011901
base:   next-20230808
patch link:    https://lore.kernel.org/r/20230808134049.1407498-3-leitao%40debian.org
patch subject: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT
config: hexagon-randconfig-r041-20230808 (https://download.01.org/0day-ci/archive/20230809/202308091103.ylvbuCww-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091103.ylvbuCww-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091103.ylvbuCww-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: sk_getsockopt
   >>> referenced by uring_cmd.c
   >>>               io_uring/uring_cmd.o:(io_uring_cmd_sock) in archive vmlinux.a
   >>> referenced by uring_cmd.c
   >>>               io_uring/uring_cmd.o:(io_uring_cmd_sock) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20230808]
[cannot apply to bpf-next/master bpf/master net/main net-next/main linus/master horms-ipvs/master v6.5-rc5 v6.5-rc4 v6.5-rc3 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/net-expose-sock_use_custom_sol_socket/20230809-011901
base:   next-20230808
patch link:    https://lore.kernel.org/r/20230808134049.1407498-8-leitao%40debian.org
patch subject: [PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd
config: x86_64-randconfig-r012-20230808 (https://download.01.org/0day-ci/archive/20230809/202308091149.ltz0y4QZ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091149.ltz0y4QZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091149.ltz0y4QZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/export.h:5,
                    from include/linux/linkage.h:7,
                    from include/linux/kernel.h:17,
                    from io_uring/uring_cmd.c:2:
   io_uring/uring_cmd.c: In function 'io_uring_cmd_getsockopt':
>> include/linux/bpf-cgroup.h:393:41: error: 'tcp_bpf_bypass_getsockopt' undeclared (first use in this function)
     393 |                                         tcp_bpf_bypass_getsockopt,             \
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:76:45: note: in definition of macro 'likely'
      76 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^
   include/linux/indirect_call_wrapper.h:66:42: note: in expansion of macro 'INDIRECT_CALL_1'
      66 | #define INDIRECT_CALL_INET_1(f, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
         |                                          ^~~~~~~~~~~~~~~
   include/linux/bpf-cgroup.h:392:22: note: in expansion of macro 'INDIRECT_CALL_INET_1'
     392 |                     !INDIRECT_CALL_INET_1((sock)->sk_prot->bpf_bypass_getsockopt, \
         |                      ^~~~~~~~~~~~~~~~~~~~
   io_uring/uring_cmd.c:191:23: note: in expansion of macro 'BPF_CGROUP_RUN_PROG_GETSOCKOPT'
     191 |                 err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf-cgroup.h:393:41: note: each undeclared identifier is reported only once for each function it appears in
     393 |                                         tcp_bpf_bypass_getsockopt,             \
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:76:45: note: in definition of macro 'likely'
      76 | # define likely(x)      __builtin_expect(!!(x), 1)
         |                                             ^
   include/linux/indirect_call_wrapper.h:66:42: note: in expansion of macro 'INDIRECT_CALL_1'
      66 | #define INDIRECT_CALL_INET_1(f, f1, ...) INDIRECT_CALL_1(f, f1, __VA_ARGS__)
         |                                          ^~~~~~~~~~~~~~~
   include/linux/bpf-cgroup.h:392:22: note: in expansion of macro 'INDIRECT_CALL_INET_1'
     392 |                     !INDIRECT_CALL_INET_1((sock)->sk_prot->bpf_bypass_getsockopt, \
         |                      ^~~~~~~~~~~~~~~~~~~~
   io_uring/uring_cmd.c:191:23: note: in expansion of macro 'BPF_CGROUP_RUN_PROG_GETSOCKOPT'
     191 |                 err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/net/sock.h:62,
                    from include/linux/bpf-cgroup.h:11,
                    from io_uring/uring_cmd.c:9:
>> include/linux/bpf-cgroup.h:393:41: error: implicit declaration of function 'tcp_bpf_bypass_getsockopt' [-Werror=implicit-function-declaration]
     393 |                                         tcp_bpf_bypass_getsockopt,             \
         |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/indirect_call_wrapper.h:19:35: note: in definition of macro 'INDIRECT_CALL_1'
      19 |                 likely(f == f1) ? f1(__VA_ARGS__) : f(__VA_ARGS__);     \
         |                                   ^~
   include/linux/bpf-cgroup.h:392:22: note: in expansion of macro 'INDIRECT_CALL_INET_1'
     392 |                     !INDIRECT_CALL_INET_1((sock)->sk_prot->bpf_bypass_getsockopt, \
         |                      ^~~~~~~~~~~~~~~~~~~~
   io_uring/uring_cmd.c:191:23: note: in expansion of macro 'BPF_CGROUP_RUN_PROG_GETSOCKOPT'
     191 |                 err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   io_uring/uring_cmd.c: In function 'io_uring_cmd_setsockopt':
   io_uring/uring_cmd.c:223:58: error: 'koptval' undeclared (first use in this function); did you mean 'optval'?
     223 |                                             USER_SOCKPTR(koptval), optlen);
         |                                                          ^~~~~~~
         |                                                          optval
   cc1: some warnings being treated as errors


vim +/tcp_bpf_bypass_getsockopt +393 include/linux/bpf-cgroup.h

0d01da6afc5402 Stanislav Fomichev 2019-06-27  384  
0d01da6afc5402 Stanislav Fomichev 2019-06-27  385  #define BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock, level, optname, optval, optlen,   \
0d01da6afc5402 Stanislav Fomichev 2019-06-27  386  				       max_optlen, retval)		       \
0d01da6afc5402 Stanislav Fomichev 2019-06-27  387  ({									       \
0d01da6afc5402 Stanislav Fomichev 2019-06-27  388  	int __ret = retval;						       \
46531a30364bd4 Pavel Begunkov     2022-01-27  389  	if (cgroup_bpf_enabled(CGROUP_GETSOCKOPT) &&			       \
46531a30364bd4 Pavel Begunkov     2022-01-27  390  	    cgroup_bpf_sock_enabled(sock, CGROUP_GETSOCKOPT))		       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  391  		if (!(sock)->sk_prot->bpf_bypass_getsockopt ||		       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  392  		    !INDIRECT_CALL_INET_1((sock)->sk_prot->bpf_bypass_getsockopt, \
9cacf81f816111 Stanislav Fomichev 2021-01-15 @393  					tcp_bpf_bypass_getsockopt,	       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  394  					level, optname))		       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  395  			__ret = __cgroup_bpf_run_filter_getsockopt(	       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  396  				sock, level, optname, optval, optlen,	       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  397  				max_optlen, retval);			       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  398  	__ret;								       \
9cacf81f816111 Stanislav Fomichev 2021-01-15  399  })
9cacf81f816111 Stanislav Fomichev 2021-01-15  400  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20230808]
[cannot apply to bpf-next/master bpf/master net/main net-next/main linus/master horms-ipvs/master v6.5-rc5 v6.5-rc4 v6.5-rc3 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/net-expose-sock_use_custom_sol_socket/20230809-011901
base:   next-20230808
patch link:    https://lore.kernel.org/r/20230808134049.1407498-4-leitao%40debian.org
patch subject: [PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT
config: hexagon-randconfig-r041-20230808 (https://download.01.org/0day-ci/archive/20230809/202308091352.OnDIGFfN-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091352.OnDIGFfN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091352.OnDIGFfN-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: sock_setsockopt
   >>> referenced by uring_cmd.c
   >>>               io_uring/uring_cmd.o:(io_uring_cmd_sock) in archive vmlinux.a
   >>> referenced by uring_cmd.c
   >>>               io_uring/uring_cmd.o:(io_uring_cmd_sock) in archive vmlinux.a

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/8] net: expose sock_use_custom_sol_socket

[Breno Leitao]

On Tue, Aug 08, 2023 at 01:46:47PM -0400, Hugo Villeneuve wrote:
> On Tue, 8 Aug 2023 10:21:03 -0700
> Breno Leitao <leitao@debian.org> wrote:
> 
> > Hello  Hugo,
> > 
> > On Tue, Aug 08, 2023 at 12:13:23PM -0400, Hugo Villeneuve wrote:
> > > On Tue,  8 Aug 2023 06:40:41 -0700
> > > Breno Leitao <leitao@debian.org> wrote:
> > > 
> > > > Exposing function sock_use_custom_sol_socket(), so it could be used by
> > > > io_uring subsystem.
> > > > 
> > > > This function will be used in the function io_uring_cmd_setsockopt() in
> > > > the coming patch, so, let's move it to the socket.h header file.
> > > 
> > > Hi,
> > > this description doesn't seem to match the code change below...
> > 
> > I re-read the patch comment and it seems to match what the code does,
> > so, probably this description only makes sense to me (?).
> > 
> > That said, hat have you understood from reading the description above?
> > socket.h
> > Thanks for the review,
> 
> Hi Breno,
> your comments says "move it to the socket.h header file" but it seems
> to be moved to the net.h header file?

Gotcha. Thanks. I will update.

Re: [PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands

[Breno Leitao]

On Tue, Aug 08, 2023 at 10:35:08AM -0700, Stanislav Fomichev wrote:
> On 08/08, Breno Leitao wrote:
> > This patchset adds support for getsockopt (SOCKET_URING_OP_GETSOCKOPT)
> > and setsockopt (SOCKET_URING_OP_SETSOCKOPT) in io_uring commands.
> > SOCKET_URING_OP_SETSOCKOPT implements generic case, covering all levels
> > nad optnames. On the other hand, SOCKET_URING_OP_GETSOCKOPT just
> > implements level SOL_SOCKET case, which seems to be the
> > most common level parameter for get/setsockopt(2).
> > 
> > struct proto_ops->setsockopt() uses sockptr instead of userspace
> > pointers, which makes it easy to bind to io_uring. Unfortunately
> > proto_ops->getsockopt() callback uses userspace pointers, except for
> > SOL_SOCKET, which is handled by sk_getsockopt(). Thus, this patchset
> > leverages sk_getsockopt() to imlpement the SOCKET_URING_OP_GETSOCKOPT
> > case.
> > 
> > In order to support BPF hooks, I modified the hooks to use  sockptr, so,
> > it is flexible enough to accept user or kernel pointers for
> > optval/optlen.
> > 
> > PS1: For getsockopt command, the optlen field is not a userspace
> > pointers, but an absolute value, so this is slightly different from
> > getsockopt(2) behaviour. The new optlen value is returned in cqe->res.
> > 
> > PS2: The userspace pointers need to be alive until the operation is
> > completed.
> > 
> > These changes were tested with a new test[1] in liburing. On the BPF
> > side, I tested that no regression was introduced by running "test_progs"
> > self test using "sockopt" test case.
> > 
> > [1] Link: https://github.com/leitao/liburing/blob/getsock/test/socket-getsetsock-cmd.c
> > 
> > RFC -> V1:
> > 	* Copy user memory at io_uring subsystem, and call proto_ops
> > 	  callbacks using kernel memory
> > 	* Implement all the cases for SOCKET_URING_OP_SETSOCKOPT
> 
> I did a quick pass, will take a close look later today. So far everything makes
> sense to me.
> 
> Should we properly test it as well?
> We have tools/testing/selftests/bpf/prog_tests/sockopt.c which does
> most of the sanity checks, but it uses regular socket/{g,s}etsockopt
> syscalls.

Right, that is what I've been using to test the changes.

> Seems like it should be pretty easy to extend this with
> io_uring path? tools/testing/selftests/net/io_uring_zerocopy_tx.c
> already implements minimal wrappers which we can most likely borrow.

Sure, I can definitely do it. Do you want to see the new tests in this
patchset, or, in a following patches?

Re: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20230808]
[cannot apply to bpf-next/master bpf/master net/main net-next/main linus/master horms-ipvs/master v6.5-rc5 v6.5-rc4 v6.5-rc3 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/net-expose-sock_use_custom_sol_socket/20230809-011901
base:   next-20230808
patch link:    https://lore.kernel.org/r/20230808134049.1407498-3-leitao%40debian.org
patch subject: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT
config: m68k-randconfig-r036-20230809 (https://download.01.org/0day-ci/archive/20230809/202308091701.sGyLMOi2-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091701.sGyLMOi2-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091701.sGyLMOi2-lkp@intel.com/

All errors (new ones prefixed by >>):

   m68k-linux-ld: io_uring/uring_cmd.o: in function `io_uring_cmd_sock':
>> io_uring/uring_cmd.c:183: undefined reference to `sk_getsockopt'


vim +183 io_uring/uring_cmd.c

   168	
   169	static inline int io_uring_cmd_getsockopt(struct socket *sock,
   170						  struct io_uring_cmd *cmd)
   171	{
   172		void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
   173		int optname = READ_ONCE(cmd->sqe->optname);
   174		int optlen = READ_ONCE(cmd->sqe->optlen);
   175		int level = READ_ONCE(cmd->sqe->level);
   176		int err;
   177	
   178		err = security_socket_getsockopt(sock, level, optname);
   179		if (err)
   180			return err;
   181	
   182		if (level == SOL_SOCKET) {
 > 183			err = sk_getsockopt(sock->sk, level, optname,
   184					    USER_SOCKPTR(optval),
   185					    KERNEL_SOCKPTR(&optlen));
   186			if (err)
   187				return err;
   188	
   189			return optlen;
   190		}
   191	
   192		return -EOPNOTSUPP;
   193	}
   194	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT

[kernel test robot]

Hi Breno,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20230808]
[cannot apply to bpf-next/master bpf/master net/main net-next/main linus/master horms-ipvs/master v6.5-rc5 v6.5-rc4 v6.5-rc3 v6.5-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Breno-Leitao/net-expose-sock_use_custom_sol_socket/20230809-011901
base:   next-20230808
patch link:    https://lore.kernel.org/r/20230808134049.1407498-4-leitao%40debian.org
patch subject: [PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT
config: m68k-randconfig-r036-20230809 (https://download.01.org/0day-ci/archive/20230809/202308091848.hWtNtas3-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230809/202308091848.hWtNtas3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308091848.hWtNtas3-lkp@intel.com/

All errors (new ones prefixed by >>):

   m68k-linux-ld: io_uring/uring_cmd.o: in function `io_uring_cmd_sock':
   io_uring/uring_cmd.c:183: undefined reference to `sk_getsockopt'
>> m68k-linux-ld: io_uring/uring_cmd.c:210: undefined reference to `sock_setsockopt'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

RE: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT

[Willem de Bruijn]

Breno Leitao wrote:
> Add support for getsockopt command (SOCKET_URING_OP_GETSOCKOPT), where
> level is SOL_SOCKET. This is leveraging the sockptr_t infrastructure,
> where a sockptr_t is either userspace or kernel space, and handled as
> such.
> 
> Function io_uring_cmd_getsockopt() is inspired by __sys_getsockopt().
> 
> Differently from the getsockopt(2), the optlen field is not a userspace
> pointers. In getsockopt(2), userspace provides optlen pointer, which is
> overwritten by the kernel.  In this implementation, userspace passes a
> u32, and the new value is returned in cqe->res. I.e., optlen is not a
> pointer.
> 
> Important to say that userspace needs to keep the pointer alive until
> the CQE is completed.

What bad things can happen otherwise?

The kernel is not depending on a well behaved process for its
correctness here, is it? Any user pages have to be pinned while
kernel might refer to them, for instance.
 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Re: [PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands

[Stanislav Fomichev]

On Wed, Aug 9, 2023 at 2:41 AM Breno Leitao <leitao@debian.org> wrote:
>
> On Tue, Aug 08, 2023 at 10:35:08AM -0700, Stanislav Fomichev wrote:
> > On 08/08, Breno Leitao wrote:
> > > This patchset adds support for getsockopt (SOCKET_URING_OP_GETSOCKOPT)
> > > and setsockopt (SOCKET_URING_OP_SETSOCKOPT) in io_uring commands.
> > > SOCKET_URING_OP_SETSOCKOPT implements generic case, covering all levels
> > > nad optnames. On the other hand, SOCKET_URING_OP_GETSOCKOPT just
> > > implements level SOL_SOCKET case, which seems to be the
> > > most common level parameter for get/setsockopt(2).
> > >
> > > struct proto_ops->setsockopt() uses sockptr instead of userspace
> > > pointers, which makes it easy to bind to io_uring. Unfortunately
> > > proto_ops->getsockopt() callback uses userspace pointers, except for
> > > SOL_SOCKET, which is handled by sk_getsockopt(). Thus, this patchset
> > > leverages sk_getsockopt() to imlpement the SOCKET_URING_OP_GETSOCKOPT
> > > case.
> > >
> > > In order to support BPF hooks, I modified the hooks to use  sockptr, so,
> > > it is flexible enough to accept user or kernel pointers for
> > > optval/optlen.
> > >
> > > PS1: For getsockopt command, the optlen field is not a userspace
> > > pointers, but an absolute value, so this is slightly different from
> > > getsockopt(2) behaviour. The new optlen value is returned in cqe->res.
> > >
> > > PS2: The userspace pointers need to be alive until the operation is
> > > completed.
> > >
> > > These changes were tested with a new test[1] in liburing. On the BPF
> > > side, I tested that no regression was introduced by running "test_progs"
> > > self test using "sockopt" test case.
> > >
> > > [1] Link: https://github.com/leitao/liburing/blob/getsock/test/socket-getsetsock-cmd.c
> > >
> > > RFC -> V1:
> > >     * Copy user memory at io_uring subsystem, and call proto_ops
> > >       callbacks using kernel memory
> > >     * Implement all the cases for SOCKET_URING_OP_SETSOCKOPT
> >
> > I did a quick pass, will take a close look later today. So far everything makes
> > sense to me.
> >
> > Should we properly test it as well?
> > We have tools/testing/selftests/bpf/prog_tests/sockopt.c which does
> > most of the sanity checks, but it uses regular socket/{g,s}etsockopt
> > syscalls.
>
> Right, that is what I've been using to test the changes.
>
> > Seems like it should be pretty easy to extend this with
> > io_uring path? tools/testing/selftests/net/io_uring_zerocopy_tx.c
> > already implements minimal wrappers which we can most likely borrow.
>
> Sure, I can definitely do it. Do you want to see the new tests in this
> patchset, or, in a following patches?

Let's keep it in the same series if possible?

Re: [PATCH v2 4/8] io_uring/cmd: Extend support beyond SOL_SOCKET

[Gabriel Krisman Bertazi]

Breno Leitao <leitao@debian.org> writes:

> Add generic support for SOCKET_URING_OP_SETSOCKOPT, expanding the
> current case, that just execute if level is SOL_SOCKET.
>
> This implementation basically calls sock->ops->setsockopt() with a
> kernel allocated optval;
>
> Since the callback for ops->setsockopt() is already using sockptr_t,
> then the callbacks are leveraged to be called directly, similarly to
> __sys_setsockopt().
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  io_uring/uring_cmd.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 5404b788ca14..dbba005a7290 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -205,10 +205,14 @@ static inline int io_uring_cmd_setsockopt(struct socket *sock,
>  	if (err)
>  		return err;
>  
> -	err = -EOPNOTSUPP;
>  	if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
>  		err = sock_setsockopt(sock, level, optname,
>  				      USER_SOCKPTR(optval), optlen);
> +	else if (unlikely(!sock->ops->setsockopt))
> +		err = -EOPNOTSUPP;
> +	else
> +		err = sock->ops->setsockopt(sock, level, optname,
> +					    USER_SOCKPTR(koptval), optlen);

Perhaps move this logic into a helper in net/ so io_uring doesn't need
to know details of struct socket and there is no duplication of this code
in __sys_getsockopt.

>  	return err;
>  }

-- 
Gabriel Krisman Bertazi

Re: [PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd

[Gabriel Krisman Bertazi]

Breno Leitao <leitao@debian.org> writes:

> Add BPF hooks support for getsockopts io_uring command. So, bpf cgroups
> programs can run when SOCKET_URING_OP_GETSOCKOPT command is called.
>
> This implementation follows a similar approach to what
> __sys_getsockopt() does, but, using USER_SOCKPTR() for optval instead of
> kernel pointer.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  io_uring/uring_cmd.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index dbba005a7290..3693e5779229 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -5,6 +5,8 @@
>  #include <linux/io_uring.h>
>  #include <linux/security.h>
>  #include <linux/nospec.h>
> +#include <linux/compat.h>
> +#include <linux/bpf-cgroup.h>
>  
>  #include <uapi/linux/io_uring.h>
>  #include <uapi/asm-generic/ioctls.h>
> @@ -179,17 +181,23 @@ static inline int io_uring_cmd_getsockopt(struct socket *sock,
>  	if (err)
>  		return err;
>  
> -	if (level == SOL_SOCKET) {
> +	err = -EOPNOTSUPP;
> +	if (level == SOL_SOCKET)
>  		err = sk_getsockopt(sock->sk, level, optname,
>  				    USER_SOCKPTR(optval),
>  				    KERNEL_SOCKPTR(&optlen));
> -		if (err)
> -			return err;
>  
> +	if (!in_compat_syscall())
> +		err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
> +						     optname,
> +						     USER_SOCKPTR(optval),
> +						     KERNEL_SOCKPTR(&optlen),
> +						     optlen, err);

I'm not sure if it makes sense to use in_compat_syscall() here.  Can't
this be invoked in a ring with ctx->compat set, but from outside a
compat syscall context (i.e. from sqpoll or even a !compat
io_uring_enter syscall)? I suspect you might need to check ctx->compact
instead, but I'm not sure. Did you consider that?

> +
> +	if (!err)
>  		return optlen;
> -	}
>  
> -	return -EOPNOTSUPP;
> +	return err;
>  }
>  
>  static inline int io_uring_cmd_setsockopt(struct socket *sock,

-- 
Gabriel Krisman Bertazi

Re: [PATCH v2 8/8] io_uring/cmd: BPF hook for setsockopt cmd

[Martin KaFai Lau]

On 8/8/23 6:40 AM, Breno Leitao wrote:
> Add support for BPF hooks for io_uring setsockopts command.
> 
> This implementation follows a similar approach to what
> __sys_setsockopt() does, but, operates only on kernel memory instead of
> user memory (which is also possible, but not preferred since the kernel
> memory is already available)
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>   io_uring/uring_cmd.c | 23 +++++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 3693e5779229..b7b27e4dbddd 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -205,23 +205,42 @@ static inline int io_uring_cmd_setsockopt(struct socket *sock,
>   {
>   	void __user *optval = u64_to_user_ptr(READ_ONCE(cmd->sqe->optval));
>   	int optname = READ_ONCE(cmd->sqe->optname);
> +	sockptr_t optval_s = USER_SOCKPTR(optval);
>   	int optlen = READ_ONCE(cmd->sqe->optlen);
>   	int level = READ_ONCE(cmd->sqe->level);
> +	char *kernel_optval = NULL;
>   	int err;
>   
>   	err = security_socket_setsockopt(sock, level, optname);
>   	if (err)
>   		return err;
>   
> +	if (!in_compat_syscall()) {
> +		err = BPF_CGROUP_RUN_PROG_SETSOCKOPT(sock->sk, &level,
> +						     &optname,
> +						     USER_SOCKPTR(optval),
> +						     &optlen,
> +						     &kernel_optval);
> +		if (err < 0)
> +			return err;
> +		if (err > 0)
> +			return 0;
> +
> +		/* Replace optval by the one returned by BPF */
> +		if (kernel_optval)
> +			optval_s = KERNEL_SOCKPTR(kernel_optval);
> +	}
> +
>   	if (level == SOL_SOCKET && !sock_use_custom_sol_socket(sock))
>   		err = sock_setsockopt(sock, level, optname,
> -				      USER_SOCKPTR(optval), optlen);
> +				      optval_s, optlen);
>   	else if (unlikely(!sock->ops->setsockopt))
>   		err = -EOPNOTSUPP;
>   	else
>   		err = sock->ops->setsockopt(sock, level, optname,
> -					    USER_SOCKPTR(koptval), optlen);
> +					    optval_s, optlen);

The bpf side changes make sense. Thanks.

With all the bpf pieces in place, __sys_{get,set}sockopt() is looking very 
similar to io_uring_cmd_{get,set}sockopt(). There are small differences like one 
takes fd and another already has a sock ptr, and io_uring_cmd_getsockopt() is 
SOL_SOCKET only. In general, can they be refactored somehow such that future 
changes don't have to be made in multiple places?

Re: [PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd

[Breno Leitao]

Hello Gabriel,

On Wed, Aug 09, 2023 at 12:46:27PM -0400, Gabriel Krisman Bertazi wrote:
> Breno Leitao <leitao@debian.org> writes:
> 
> > Add BPF hooks support for getsockopts io_uring command. So, bpf cgroups
> > programs can run when SOCKET_URING_OP_GETSOCKOPT command is called.
> >
> > This implementation follows a similar approach to what
> > __sys_getsockopt() does, but, using USER_SOCKPTR() for optval instead of
> > kernel pointer.
> >
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> > ---
> >  io_uring/uring_cmd.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> > index dbba005a7290..3693e5779229 100644
> > --- a/io_uring/uring_cmd.c
> > +++ b/io_uring/uring_cmd.c
> > @@ -5,6 +5,8 @@
> >  #include <linux/io_uring.h>
> >  #include <linux/security.h>
> >  #include <linux/nospec.h>
> > +#include <linux/compat.h>
> > +#include <linux/bpf-cgroup.h>
> >  
> >  #include <uapi/linux/io_uring.h>
> >  #include <uapi/asm-generic/ioctls.h>
> > @@ -179,17 +181,23 @@ static inline int io_uring_cmd_getsockopt(struct socket *sock,
> >  	if (err)
> >  		return err;
> >  
> > -	if (level == SOL_SOCKET) {
> > +	err = -EOPNOTSUPP;
> > +	if (level == SOL_SOCKET)
> >  		err = sk_getsockopt(sock->sk, level, optname,
> >  				    USER_SOCKPTR(optval),
> >  				    KERNEL_SOCKPTR(&optlen));
> > -		if (err)
> > -			return err;
> >  
> > +	if (!in_compat_syscall())
> > +		err = BPF_CGROUP_RUN_PROG_GETSOCKOPT(sock->sk, level,
> > +						     optname,
> > +						     USER_SOCKPTR(optval),
> > +						     KERNEL_SOCKPTR(&optlen),
> > +						     optlen, err);
> 
> I'm not sure if it makes sense to use in_compat_syscall() here.  Can't
> this be invoked in a ring with ctx->compat set, but from outside a
> compat syscall context (i.e. from sqpoll or even a !compat
> io_uring_enter syscall)? I suspect you might need to check ctx->compact
> instead, but I'm not sure. Did you consider that?

I think that checking ctx->compat seems to be the right thing to do. I
will update.

Re: [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT

[Pavel Begunkov]

On 8/9/23 14:21, Willem de Bruijn wrote:
> Breno Leitao wrote:
>> Add support for getsockopt command (SOCKET_URING_OP_GETSOCKOPT), where
>> level is SOL_SOCKET. This is leveraging the sockptr_t infrastructure,
>> where a sockptr_t is either userspace or kernel space, and handled as
>> such.
>>
>> Function io_uring_cmd_getsockopt() is inspired by __sys_getsockopt().
>>
>> Differently from the getsockopt(2), the optlen field is not a userspace
>> pointers. In getsockopt(2), userspace provides optlen pointer, which is
>> overwritten by the kernel.  In this implementation, userspace passes a
>> u32, and the new value is returned in cqe->res. I.e., optlen is not a
>> pointer.
>>
>> Important to say that userspace needs to keep the pointer alive until
>> the CQE is completed.
> 
> What bad things can happen otherwise?
> 
> The kernel is not depending on a well behaved process for its
> correctness here, is it? Any user pages have to be pinned while

Right, it's the user api thing. There are always userspace progs
that would try to do:

submit_async() {
	char buf[20];
	do_submit(sqe = {buf = buf, ...});
}

submit_async();
wait_completions();


> kernel might refer to them, for instance.

fwiw, it's passed down as a user ptr, which will be eventually
used in copy_[from,to]_user() or so.

-- 
Pavel Begunkov