| Message ID | 20230808134049.1407498-3-leitao@debian.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: Initial support for {s,g}etsockopt commands | expand |
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
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
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>
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.
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; }
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(+)