netdev - Re: [PATCH bpf-next 1/2] bpf: add hardware timestamp field to __sk

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]

Message-ID: <fd2e2457-b3c2-81a4-481a-27555e4473dc@iogearbox.net>
Date:   Fri, 3 Sep 2021 10:22:58 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Vadim Fedorenko <vfedorenko@...ek.ru>,
        Martin KaFai Lau <kafai@...com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        bpf@...r.kernel.org, willemb@...gle.com
Subject: Re: [PATCH bpf-next 1/2] bpf: add hardware timestamp field to
 __sk_buff

On 9/3/21 12:15 AM, Vadim Fedorenko wrote:
> BPF programs may want to know hardware timestamps if NIC supports
> such timestamping.
> 
> Expose this data as hwtstamp field of __sk_buff the same way as
> gso_segs/gso_size.
> 
> Also update BPF_PROG_TEST_RUN tests of the feature.
> 
> Signed-off-by: Vadim Fedorenko <vfedorenko@...ek.ru>
> ---
>   include/uapi/linux/bpf.h       |  2 ++
>   net/core/filter.c              | 11 +++++++++++
>   tools/include/uapi/linux/bpf.h |  2 ++
>   3 files changed, 15 insertions(+)
> 
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 791f31dd0abe..c7d05b49f557 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -5284,6 +5284,8 @@ struct __sk_buff {
>   	__u32 gso_segs;
>   	__bpf_md_ptr(struct bpf_sock *, sk);
>   	__u32 gso_size;
> +	__u32 padding;		/* Padding, future use. */

nit, instead of explicit padding field, just use: __u32 :32;

Also please add test_verifier coverage for this in BPF selftests, meaning,
the expectation would be in case someone tries to access the padding field
with this patch that we get a 'bpf verifier is misconfigured' error given
it would have no bpf_convert_ctx_access() translation. But it would be overall
better to add this to bpf_skb_is_valid_access(), so we can reject access to
the padding area right there instead.

> +	__u64 hwtstamp;
>   };
>   
>   struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 2e32cee2c469..1d8f8494d325 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -8884,6 +8884,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type type,
>   				      si->dst_reg, si->src_reg,
>   				      offsetof(struct sk_buff, sk));
>   		break;
> +	case offsetof(struct __sk_buff, hwtstamp):
> +		BUILD_BUG_ON(sizeof_field(struct skb_shared_hwtstamps, hwtstamp) != 8);
> +		BUILD_BUG_ON(offsetof(struct skb_shared_hwtstamps, hwtstamp) != 0);
> +
> +		insn = bpf_convert_shinfo_access(si, insn);
> +		*insn++ = BPF_LDX_MEM(BPF_DW,
> +				      si->dst_reg, si->dst_reg,
> +				      bpf_target_off(struct skb_shared_info,
> +						     hwtstamps, 8,
> +						     target_size));
> +		break;
>   	}
>   
>   	return insn - insn_buf;
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 791f31dd0abe..c7d05b49f557 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -5284,6 +5284,8 @@ struct __sk_buff {
>   	__u32 gso_segs;
>   	__bpf_md_ptr(struct bpf_sock *, sk);
>   	__u32 gso_size;
> +	__u32 padding;		/* Padding, future use. */
> +	__u64 hwtstamp;
>   };
>   
>   struct bpf_tunnel_key {
>