Re: [PATCH mptcp-next v4 4/5] Squash to "bpf: Add mptcp_subflow bpf_iter"

[Matthieu Baerts <matttbe@kernel.org>]

Hi Geliang,

On 09/12/2024 09:47, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> Add "sizeof" and "alignof" checks.
> 
> Address Martin's comments in v1:
> 
> - bpf_iter_mptcp_subflow_new returns -EINVAL when msk socket lock isn't
>   held.
> 
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
>  net/mptcp/bpf.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index f9ba0a46a9f1..0563d3c6d9d3 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -232,12 +232,20 @@ bpf_iter_mptcp_subflow_new(struct bpf_iter_mptcp_subflow *it,
>  			   struct mptcp_sock *msk)
>  {
>  	struct bpf_iter_mptcp_subflow_kern *kit = (void *)it;
> +	struct sock *sk = (struct sock *)msk;
> +
> +	BUILD_BUG_ON(sizeof(struct bpf_iter_mptcp_subflow_kern) >
> +		     sizeof(struct bpf_iter_mptcp_subflow));
> +	BUILD_BUG_ON(__alignof__(struct bpf_iter_mptcp_subflow_kern) !=
> +		     __alignof__(struct bpf_iter_mptcp_subflow));
>  
>  	kit->msk = msk;
>  	if (!msk)
>  		return -EINVAL;
>  
> -	msk_owned_by_me(msk);
> +	if (!sock_owned_by_user_nocheck(sk) &&
> +	    !spin_is_locked(&sk->sk_lock.slock))
> +		return -EINVAL;

Just to be sure, did you check it was always the case with the WIP
scheduler and path-manager BPF programs you have?

We don't want to be too restrictive. (Or we need different helpers)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.