All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin KaFai Lau <martin.lau@linux.dev>
To: Breno Leitao <leitao@debian.org>
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, io-uring@vger.kernel.org,
	kuba@kernel.org, pabeni@redhat.com, sdf@google.com,
	axboe@kernel.dk, asml.silence@gmail.com,
	willemdebruijn.kernel@gmail.com
Subject: Re: [PATCH v2 8/8] io_uring/cmd: BPF hook for setsockopt cmd
Date: Wed, 9 Aug 2023 15:02:31 -0700	[thread overview]
Message-ID: <afd0c815-e32f-6b57-a72b-4bcd46b78136@linux.dev> (raw)
In-Reply-To: <20230808134049.1407498-9-leitao@debian.org>

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?


  reply	other threads:[~2023-08-09 22:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-08 13:40 [PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands Breno Leitao
2023-08-08 13:40 ` [PATCH v2 1/8] net: expose sock_use_custom_sol_socket Breno Leitao
2023-08-08 16:13   ` Hugo Villeneuve
2023-08-08 17:21     ` Breno Leitao
2023-08-08 17:46       ` Hugo Villeneuve
2023-08-09  9:39         ` Breno Leitao
2023-08-08 20:12       ` Jeff Moyer
2023-08-08 13:40 ` [PATCH v2 2/8] io_uring/cmd: Introduce SOCKET_URING_OP_GETSOCKOPT Breno Leitao
2023-08-09  4:07   ` kernel test robot
2023-08-09 10:27   ` kernel test robot
2023-08-09 13:21   ` Willem de Bruijn
2023-08-10 12:57     ` Pavel Begunkov
2023-08-08 13:40 ` [PATCH v2 3/8] io_uring/cmd: Introduce SOCKET_URING_OP_SETSOCKOPT Breno Leitao
2023-08-09  6:01   ` kernel test robot
2023-08-09 11:09   ` kernel test robot
2023-08-08 13:40 ` [PATCH v2 4/8] io_uring/cmd: Extend support beyond SOL_SOCKET Breno Leitao
2023-08-09 16:32   ` Gabriel Krisman Bertazi
2023-08-08 13:40 ` [PATCH v2 5/8] bpf: Leverage sockptr_t in BPF getsockopt hook Breno Leitao
2023-08-08 13:40 ` [PATCH v2 6/8] bpf: Leverage sockptr_t in BPF setsockopt hook Breno Leitao
2023-08-08 13:40 ` [PATCH v2 7/8] io_uring/cmd: BPF hook for getsockopt cmd Breno Leitao
2023-08-09  4:17   ` kernel test robot
2023-08-09 16:46   ` Gabriel Krisman Bertazi
2023-08-10  8:26     ` Breno Leitao
2023-08-08 13:40 ` [PATCH v2 8/8] io_uring/cmd: BPF hook for setsockopt cmd Breno Leitao
2023-08-09 22:02   ` Martin KaFai Lau [this message]
2023-08-08 17:35 ` [PATCH v2 0/8] io_uring: Initial support for {s,g}etsockopt commands Stanislav Fomichev
2023-08-09  9:40   ` Breno Leitao
2023-08-09 16:26     ` Stanislav Fomichev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=afd0c815-e32f-6b57-a72b-4bcd46b78136@linux.dev \
    --to=martin.lau@linux.dev \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=bpf@vger.kernel.org \
    --cc=io-uring@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=leitao@debian.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@google.com \
    --cc=willemdebruijn.kernel@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.