Re: [PATCH] bpf: Fix mix-up of 4096 and page size.

[Alexander Lobakin]

From: Alexei Starovoitov <alexei.starovoitov@xxxxxxxxx>
Date: Thu, 23 Jan 2025 21:14:04 -0800

> On Wed, Jan 22, 2025 at 10:38 AM Saket Kumar Bhaskar
> <skb99@xxxxxxxxxxxxx> wrote:
>>
>> For platforms on powerpc architecture with a default page size greater
>> than 4096, there was an inconsistency in fragment size calculation.
>> This caused the BPF selftest xdp_adjust_tail/xdp_adjust_frags_tail_grow
>> to fail on powerpc.
>>
>> The issue occurred because the fragment buffer size in
>> bpf_prog_test_run_xdp() was set to 4096, while the actual data size in
>> the fragment within the shared skb was checked against PAGE_SIZE
>> (65536 on powerpc) in min_t, causing it to exceed 4096 and be set
>> accordingly. This discrepancy led to an overflow when
>> bpf_xdp_frags_increase_tail() checked for tailroom, as skb_frag_size(frag)
>> could be greater than rxq->frag_size (when PAGE_SIZE > 4096).
>>
>> This commit updates the page size references to 4096 to ensure consistency
>> and prevent overflow issues in fragment size calculations.
> 
> This isn't right. Please fix the selftest instead.

It's not _that_ easy, I had tried in the past. Anyway, this patch is
*not* a good "solution".

If you (Saket) really want to fix this, both test_run and the selftest
must be in sync, so you need to (both are arch-dependent): 1) get the
correct PAGE_SIZE; 2) calculate the correct tailroom in userspace (which
depends on sizeof(shinfo) and SKB_DATA_ALIGN -> SMP_CACHE_BYTES).

> 
> pw-bot: cr

Thanks,
Olek