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

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: <4524f1c3-a63c-1fb1-ee53-94350e0dc280@fb.com>
Date:   Sun, 29 Apr 2018 17:00:23 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     <ast@...com>, <daniel@...earbox.net>, <netdev@...r.kernel.org>,
        <kernel-team@...com>
Subject: Re: [PATCH bpf-next] samples/bpf: fix kprobe attachment issue on x64



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@...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.

That is why I took the approach as in the patch.