Re: [PATCH mptcp-next] Squash to "mptcp: add bpf_mptcp_sched_ops" -- fix bpf access

[Geliang Tang <geliang@kernel.org>]

Hi Gregory,

On Fri, May 03, 2024 at 07:33:25PM +0000, Gregory Detal wrote:
> The current behavior allows to write to mptcp_sock at offset that is
> defined in mptcp_subflow_context and vice versa.
> 
> This fixes this by splitting the checks for each struct type.
> 
> Signed-off-by: Gregory Detal <gregory.detal@gmail.com>

Thanks for this fix. Looks good to me.

Reviewed-by: Geliang Tang <geliang@kernel.org>

-Geliang

> ---
>  net/mptcp/bpf.c | 42 +++++++++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/net/mptcp/bpf.c b/net/mptcp/bpf.c
> index 208e5d3f066f..57c47bb430b1 100644
> --- a/net/mptcp/bpf.c
> +++ b/net/mptcp/bpf.c
> @@ -47,24 +47,32 @@ static int bpf_mptcp_sched_btf_struct_access(struct bpf_verifier_log *log,
>  	size_t end;
>  
>  	t = btf_type_by_id(reg->btf, reg->btf_id);
> -	if (t != mptcp_sock_type && t != mptcp_subflow_type) {
> -		bpf_log(log, "only access to mptcp sock or subflow is supported\n");
> -		return -EACCES;
> -	}
>  
> -	switch (off) {
> -	case offsetof(struct mptcp_sock, snd_burst):
> -		end = offsetofend(struct mptcp_sock, snd_burst);
> -		break;
> -	case offsetof(struct mptcp_subflow_context, scheduled):
> -		end = offsetofend(struct mptcp_subflow_context, scheduled);
> -		break;
> -	case offsetof(struct mptcp_subflow_context, avg_pacing_rate):
> -		end = offsetofend(struct mptcp_subflow_context, avg_pacing_rate);
> -		break;
> -	default:
> -		bpf_log(log, "no write support to %s at off %d\n",
> -			t == mptcp_sock_type ? "mptcp_sock" : "mptcp_subflow_context", off);
> +	if (t == mptcp_sock_type) {
> +		switch (off) {
> +		case offsetof(struct mptcp_sock, snd_burst):
> +			end = offsetofend(struct mptcp_sock, snd_burst);
> +			break;
> +		default:
> +			bpf_log(log, "no write support to mptcp_sock at off %d\n",
> +				off);
> +			return -EACCES;
> +		}
> +	} else if (t == mptcp_subflow_type) {
> +		switch (off) {
> +		case offsetof(struct mptcp_subflow_context, scheduled):
> +			end = offsetofend(struct mptcp_subflow_context, scheduled);
> +			break;
> +		case offsetof(struct mptcp_subflow_context, avg_pacing_rate):
> +			end = offsetofend(struct mptcp_subflow_context, avg_pacing_rate);
> +			break;
> +		default:
> +			bpf_log(log, "no write support to mptcp_subflow_context at off %d\n",
> +				off);
> +			return -EACCES;
> +		}
> +	} else {
> +		bpf_log(log, "only access to mptcp sock or subflow is supported\n");
>  		return -EACCES;
>  	}
>  
> 
> ---
> base-commit: 56030f9d3812071365435354c0eb5ffb3504e58a
> change-id: 20240503-bpf_fix_access-a360b88c1534
> 
> Best regards,
> -- 
> Gregory Detal <gregory.detal@gmail.com>
>