netdev - Re: [PATCH bpf-next v3 0/2] Allow data

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] [day] [month] [year] [list]

Message-ID: <c37449f1-4b23-41a7-a31d-e58cd35a6142@intel.com>
Date: Tue, 28 Nov 2023 13:25:36 +0100
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Jesper Dangaard Brouer <hawk@...nel.org>, Larysa Zaremba
	<larysa.zaremba@...el.com>
CC: <bpf@...r.kernel.org>, <netdev@...r.kernel.org>, Alexei Starovoitov
	<ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, "David S. Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Eric Dumazet
	<edumazet@...gle.com>, Magnus Karlsson <magnus.karlsson@...il.com>, "Willem
 de Bruijn" <willemdebruijn.kernel@...il.com>, Yunsheng Lin
	<linyunsheng@...wei.com>, Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
	John Fastabend <john.fastabend@...il.com>
Subject: Re: [PATCH bpf-next v3 0/2] Allow data_meta size > 32

From: Jesper Dangaard Brouer <hawk@...nel.org>
Date: Tue, 28 Nov 2023 12:30:41 +0100

> 
> 
> On 11/28/23 11:33, Larysa Zaremba wrote:
>> On Tue, Nov 28, 2023 at 11:26:59AM +0100, Jesper Dangaard Brouer wrote:
>>>
>>>
>>> On 11/27/23 19:32, Larysa Zaremba wrote:
>>>> Currently, there is no reason for data_meta to be limited to 32 bytes.
>>>> Loosen this limitation and make maximum meta size 252.
>>>
>>> First I though you made a type here with 252 bytes, but then I
>>> remembered
>>> the 4 byte alignment.
>>> I think commit message should elaborate on why 252 bytes.
>>>
>>
>> You are right, will do.
>>   
> 
> I'm looking at the code to see if metadata can be extended into area
> used by xdp_frame, such that a BPF-prog get direct memory access to this
> (which should not be allowed).  The bpf_xdp_adjust_meta() helper does
> handle this (as you/Olek also mentioned in commit msg).  A driver could
> set data_meta such that they overlap, but I guess we will categorize
> this as a driver bug.
> 
> The XDP headroom have evolved to become dynamic (commonly 192 or 256
> bytes). Thus, we cannot statically reduce metalen with sizeof(struct
> xdp_frame).  The maximum meta size 252, could be achieved (and valid) if
> a driver chooses to have more XDP headroom e.g. 288 bytes. So, I guess
> it is correct to say maximum valid meta size is 252 bytes, but can be
> limited and reduced further by drivers chosen XDP headroom and memory
> reserved by struct xdp_frame (limited in bpf_xdp_adjust_meta).

Drivers which don't provide 256 bytes of headroom for XDP are
pathological :p

Either way, as Larysa and you mentioned, the actual available headroom
is checked with adjust_meta(), so here we just make sure its size fits
into u8 meta_len of &skb_shared_info.

> 
> 
> 
>>>>
>>>> Also, modify the selftest, so test_xdp_context_error does not complain
>>>> about the unexpected success.
>>>>
>>>> v2->v3:
>>>> * Fix main patch author
>>>> * Add selftests path
>>>>
>>>> v1->v2:
>>>> * replace 'typeof(metalen)' with the actual type
>>>>
>>>> Aleksander Lobakin (1):
>>>>     net, xdp: allow metadata > 32
>>>>
>>>> Larysa Zaremba (1):
>>>>     selftests/bpf: increase invalid metadata size
>>>>
>>>>    include/linux/skbuff.h                              | 13
>>>> ++++++++-----
>>>>    include/net/xdp.h                                   |  7 ++++++-
>>>>    .../selftests/bpf/prog_tests/xdp_context_test_run.c |  4 ++--
>>>>    3 files changed, 16 insertions(+), 8 deletions(-)
>>>>

Thanks,
Olek