netdev - Re: [xdp-hints] [PATCH bpf-next RFC V1] selftests/bpf: xdp_hw

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: <87cz70krjv.fsf@toke.dk>
Date:   Fri, 27 Jan 2023 14:58:28 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>, bpf@...r.kernel.org,
        Stanislav Fomichev <sdf@...gle.com>
Cc:     Jesper Dangaard Brouer <brouer@...hat.com>, netdev@...r.kernel.org,
        martin.lau@...nel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, martin.lau@...ux.dev, song@...nel.org,
        yhs@...com, john.fastabend@...il.com, dsahern@...il.com,
        willemb@...gle.com, void@...ifault.com, kuba@...nel.org,
        xdp-hints@...-project.net
Subject: Re: [xdp-hints] [PATCH bpf-next RFC V1] selftests/bpf:
 xdp_hw_metadata clear metadata when -EOPNOTSUPP

Jesper Dangaard Brouer <brouer@...hat.com> writes:

> The AF_XDP userspace part of xdp_hw_metadata see non-zero as a signal of
> the availability of rx_timestamp and rx_hash in data_meta area. The
> kernel-side BPF-prog code doesn't initialize these members when kernel
> returns an error e.g. -EOPNOTSUPP.  This memory area is not guaranteed to
> be zeroed, and can contain garbage/previous values, which will be read
> and interpreted by AF_XDP userspace side.
>
> Tested this on different drivers. The experiences are that for most
> packets they will have zeroed this data_meta area, but occasionally it
> will contain garbage data.
>
> Example of failure tested on ixgbe:
>  poll: 1 (0)
>  xsk_ring_cons__peek: 1
>  0x18ec788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000
>  rx_hash: 3697961069
>  rx_timestamp:  9024981991734834796 (sec:9024981991.7348)
>  0x18ec788: complete idx=8 addr=8000
>
> Converting to date:
>  date -d @9024981991
>  2255-12-28T20:26:31 CET
>
> I choose a simple fix in this patch. When kfunc fails or isn't supported
> assign zero to the corresponding struct meta value.
>
> It's up to the individual BPF-programmer to do something smarter e.g.
> that fits their use-case, like getting a software timestamp and marking
> a flag that gives the type of timestamp.
>
> Another possibility is for the behavior of kfunc's
> bpf_xdp_metadata_rx_timestamp and bpf_xdp_metadata_rx_hash to require
> clearing return value pointer.

I definitely think we should leave it up to the BPF programmer to react
to failures; that's what the return code is there for, after all :)

-Toke