From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yonghong Song Subject: Re: [PATCH bpf-next] samples/bpf: fix kprobe attachment issue on x64 Date: Sun, 29 Apr 2018 17:44:31 -0700 Message-ID: References: <20180429220631.2925150-1-yhs@fb.com> <20180429232031.5p2y4s274inymf4b@ast-mbp> <4524f1c3-a63c-1fb1-ee53-94350e0dc280@fb.com> <20180430000622.p4flh2hyd2pydlz4@ast-mbp> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Cc: , , , To: Alexei Starovoitov Return-path: Received: from mx0b-00082601.pphosted.com ([67.231.153.30]:45364 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754385AbeD3ApE (ORCPT ); Sun, 29 Apr 2018 20:45:04 -0400 In-Reply-To: <20180430000622.p4flh2hyd2pydlz4@ast-mbp> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: 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 >>>> --- >>>> 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.