Re: [PATCH bpf-next] samples/bpf: fix kprobe attachment issue on x64

[Yonghong Song <yhs@fb.com>]

On 4/29/18 5:06 PM, Alexei Starovoitov wrote:
> On Sun, Apr 29, 2018 at 05:00:23PM -0700, Yonghong Song wrote:
>>
>>
>> On 4/29/18 4:20 PM, Alexei Starovoitov wrote:
>>> On Sun, Apr 29, 2018 at 03:06:31PM -0700, Yonghong Song wrote:
>>>> Commit d5a00528b58c ("syscalls/core, syscalls/x86: Rename
>>>> struct pt_regs-based sys_*() to __x64_sys_*()") renamed a lot
>>>> of syscall function sys_*() to __x64_sys_*().
>>>> This caused several kprobe based samples/bpf tests failing.
>>>>
>>>> This patch fixed the problem by using __x64_sys_*(),
>>>> instead of sys_*(), in bpf program SEC annotations if
>>>> the target arch is __TARGET_ARCH_x86.
>>>>
>>>> Fixes: d5a00528b58c ("syscalls/core, syscalls/x86: Rename struct pt_regs-based sys_*() to __x64_sys_*()")
>>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>>> ---
>>>>    samples/bpf/map_perf_test_kern.c                  | 32 +++++++++++++++++++++++
>>>>    samples/bpf/test_current_task_under_cgroup_kern.c |  4 +++
>>>>    samples/bpf/test_map_in_map_kern.c                |  4 +++
>>>>    samples/bpf/test_probe_write_user_kern.c          |  4 +++
>>>>    samples/bpf/trace_output_kern.c                   |  4 +++
>>>>    samples/bpf/tracex2_kern.c                        |  4 +++
>>>>    6 files changed, 52 insertions(+)
>>>>
>>>> diff --git a/samples/bpf/map_perf_test_kern.c b/samples/bpf/map_perf_test_kern.c
>>>> index 2b2ffb9..79f4320 100644
>>>> --- a/samples/bpf/map_perf_test_kern.c
>>>> +++ b/samples/bpf/map_perf_test_kern.c
>>>> @@ -95,7 +95,11 @@ struct bpf_map_def SEC("maps") lru_hash_lookup_map = {
>>>>    	.max_entries = MAX_ENTRIES,
>>>>    };
>>>> +#ifdef __TARGET_ARCH_x86
>>>> +SEC("kprobe/__x64_sys_getuid")
>>>> +#else
>>>>    SEC("kprobe/sys_getuid")
>>>> +#endif
>>>
>>> I think it would be better to hack bpf_load.c to add __x64_
>>> automatically when it matches "sys_" in the beginning of kprobe name.
>>
>> I thought this before but there a few outliers for the particular
>> kernel configuration I have for latest bpf-next:
>>
>> drivers/video/fbdev/core/sysfillrect.c:
>>    void sys_fillrect(struct fb_info *p, const struct fb_fillrect *rect)
>> drivers/video/fbdev/core/syscopyarea.c:
>>    void sys_copyarea(struct fb_info *p, const struct fb_copyarea *area)
>> drivers/video/fbdev/core/sysimgblt.c:
>>    void sys_imageblit(struct fb_info *p, const struct fb_image *image)
>>
>> I am not sure whether any other outliers for other configurations or
>> in the future somebody could introduces a kernel function sys_ but
>> not a syscall.
> 
> How about trying to kprobe both ?
> First __x64_sys_* and if it doesn't exist kprobe on sys_* ?

This should work. Let us do this.