Re: [REGRESSION] module BTF validation failure (Error -22) on next

[Alan Maguire <alan.maguire@oracle.com>]

On 14/12/2024 04:41, Cong Wang wrote:
> On Thu, Dec 05, 2024 at 08:36:33AM +0100, Uros Bizjak wrote:
>> On Wed, Dec 4, 2024 at 4:52 PM Laura Nao <laura.nao@collabora.com> wrote:
>>>
>>> On 11/15/24 18:17, Laura Nao wrote:
>>>> I managed to reproduce the issue locally and I've uploaded the vmlinux[1]
>>>> (stripped of DWARF data) and vmlinux.raw[2] files, as well as one of the
>>>> modules[3] and its btf data[4] extracted with:
>>>>
>>>> bpftool -B vmlinux btf dump file cros_kbd_led_backlight.ko > cros_kbd_led_backlight.ko.raw
>>>>
>>>> Looking again at the logs[5], I've noticed the following is reported:
>>>>
>>>> [    0.415885] BPF:    type_id=115803 offset=177920 size=1152
>>>> [    0.416029] BPF:
>>>> [    0.416083] BPF: Invalid offset
>>>> [    0.416165] BPF:
>>>>
>>>> There are two different definitions of rcu_data in '.data..percpu', one
>>>> is a struct and the other is an integer:
>>>>
>>>> type_id=115801 offset=177920 size=1152 (VAR 'rcu_data')
>>>> type_id=115803 offset=177920 size=1152 (VAR 'rcu_data')
>>>>
>>>> [115801] VAR 'rcu_data' type_id=115572, linkage=static
>>>> [115803] VAR 'rcu_data' type_id=1, linkage=static
>>>>
>>>> [115572] STRUCT 'rcu_data' size=1152 vlen=69
>>>> [1] INT 'long unsigned int' size=8 bits_offset=0 nr_bits=64 encoding=(none)
>>>>
>>>> I assume that's not expected, correct?
>>>>
>>>> I'll dig a bit deeper and report back if I can find anything else.
>>>
>>> I ran a bisection, and it appears the culprit commit is:
>>> https://lore.kernel.org/all/20241021080856.48746-2-ubizjak@gmail.com/
>>>
>>> Hi Uros, do you have any suggestions or insights on resolving this issue?
>>
>> There is a stray ";" at the end of the #define, perhaps this makes a difference:
>>
>> +#define PERCPU_PTR(__p) \
>> + (typeof(*(__p)) __force __kernel *)(__p);
>> +
>>
>> and SHIFT_PERCPU_PTR macro now expands to:
>>
>> RELOC_HIDE((typeof(*(p)) __force __kernel *)(p);, (offset))
>>
>> A follow-up patch in the series changes PERCPU_PTR macro to:
>>
>> #define PERCPU_PTR(__p) \
>> ({ \
>> unsigned long __pcpu_ptr = (__force unsigned long)(__p); \
>> (typeof(*(__p)) __force __kernel *)(__pcpu_ptr); \
>> })
>>
>> so this should again correctly cast the value.
> 
> Hm, I saw a similar bug but with pahole 1.28. My kernel complains about
> BTF invalid offset:
> 
> [    7.785788] BPF: 	 type_id=2394 offset=0 size=1
> [    7.786411] BPF:
> [    7.786703] BPF: Invalid offset
> [    7.787119] BPF:
> 
> Dumping the vmlinux (there is no module invovled), I saw it is related to
> percpu pointer too:
> 
> [2394] VAR '__pcpu_unique_cpu_hw_events' type_id=2, linkage=global
> ...
> [163643] DATASEC '.data..percpu' size=2123280 vlen=808
>         type_id=2393 offset=0 size=1 (VAR '__pcpu_scope_cpu_hw_events')
>         type_id=2394 offset=0 size=1 (VAR '__pcpu_unique_cpu_hw_events')
> ...
> 
> I compiled and installed the latest pahole from its git repo:
> 
> $ pahole --version
> v1.28
> 
> Thanks.

Thanks for the report! Looking at percpu-defs.h it looks like the
existence of such variables requires either

#if defined(ARCH_NEEDS_WEAK_PER_CPU) ||
defined(CONFIG_DEBUG_FORCE_WEAK_PER_CPU)

...

#define DEFINE_PER_CPU_SECTION(type, name, sec)                         \
        __PCPU_DUMMY_ATTRS char __pcpu_scope_##name;                    \
        extern __PCPU_DUMMY_ATTRS char __pcpu_unique_##name;            \
        __PCPU_DUMMY_ATTRS char __pcpu_unique_##name;                   \
        extern __PCPU_ATTRS(sec) __typeof__(type) name;                 \
        __PCPU_ATTRS(sec) __weak __typeof__(type) name


I'm guessing your .config has CONFIG_DEBUG_FORCE_WEAK_PER_CPU, or are
you building on s390/alpha?

I've reproduced this on bpf-next with CONFIG_DEBUG_FORCE_WEAK_PER_CPU=y,
pahole v1.28 and gcc-12; I see ~900 __pcpu_ variables and get the same
BTF errors since multipe __pcpu_ vars share the offset 0.

A simple workaround in dwarves - and I verified this resolved the issue
for me - would be

diff --git a/btf_encoder.c b/btf_encoder.c
index 3754884..4a1799a 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -2174,7 +2174,8 @@ static bool filter_variable_name(const char *name)
                X("__UNIQUE_ID"),
                X("__tpstrtab_"),
                X("__exitcall_"),
-               X("__func_stack_frame_non_standard_")
+               X("__func_stack_frame_non_standard_"),
+               X("__pcpu_")
                #undef X
        };
        int i;

...but I'd like us to understand further why variables which were
supposed to be in a .discard section end up being encoded as there may
be other problems lurking here aside from this one. More soon hopefully...

Alan