netdev - Re: [PATCH bpf-next v3] Add skb_store_bytes() for BPF_PROG_TYPE_CGROUP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 7 Jan 2022 00:34:39 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Tyler Wear <quic_twear@...cinc.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Cc:     maze@...gle.com, yhs@...com, kafai@...com, toke@...hat.com,
        Tyler Wear <quic_twear@...cinc.org>
Subject: Re: [PATCH bpf-next v3] Add skb_store_bytes() for
 BPF_PROG_TYPE_CGROUP_SKB

On 1/6/22 1:43 AM, Tyler Wear wrote:
> From: Tyler Wear <quic_twear@...cinc.org>
> 
> Need to modify the ds field to support upcoming Wifi QoS Alliance spec.
> Instead of adding generic function for just modifying the ds field,
> add skb_store_bytes for BPF_PROG_TYPE_CGROUP_SKB.
> This allows other fields in the network and transport header to be
> modified in the future.
> 
> Checksum API's also need to be added for completeness.
> 
> It is not possible to use CGROUP_(SET|GET)SOCKOPT since
> the policy may change during runtime and would result
> in a large number of entries with wildcards.
> 
> Signed-off-by: Tyler Wear <quic_twear@...cinc.com>
> ---
>   net/core/filter.c                             | 10 ++
>   .../bpf/prog_tests/cgroup_store_bytes.c       | 97 +++++++++++++++++++
>   .../selftests/bpf/progs/cgroup_store_bytes.c  | 64 ++++++++++++
>   3 files changed, 171 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/prog_tests/cgroup_store_bytes.c
>   create mode 100644 tools/testing/selftests/bpf/progs/cgroup_store_bytes.c
> 
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6102f093d59a..ce01a8036361 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -7299,6 +7299,16 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>   		return &bpf_sk_storage_delete_proto;
>   	case BPF_FUNC_perf_event_output:
>   		return &bpf_skb_event_output_proto;
> +	case BPF_FUNC_skb_store_bytes:
> +		return &bpf_skb_store_bytes_proto;
> +	case BPF_FUNC_csum_update:
> +		return &bpf_csum_update_proto;
> +	case BPF_FUNC_csum_level:
> +		return &bpf_csum_level_proto;
> +	case BPF_FUNC_l3_csum_replace:
> +		return &bpf_l3_csum_replace_proto;
> +	case BPF_FUNC_l4_csum_replace:
> +		return &bpf_l4_csum_replace_proto;
>   #ifdef CONFIG_SOCK_CGROUP_DATA
>   	case BPF_FUNC_skb_cgroup_id:
>   		return &bpf_skb_cgroup_id_proto;

Do we need skb_share_check in the write helpers at these hook points when this
goes beyond just reading?

Thanks,
Daniel