From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: Re: [PATCH bpf-next] samples/bpf: fix kprobe attachment issue on x64
Date: Sun, 29 Apr 2018 17:06:24 -0700
Message-ID: <20180430000622.p4flh2hyd2pydlz4@ast-mbp>
References: <20180429220631.2925150-1-yhs@fb.com>
 <20180429232031.5p2y4s274inymf4b@ast-mbp>
 <4524f1c3-a63c-1fb1-ee53-94350e0dc280@fb.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: ast@fb.com, daniel@iogearbox.net, netdev@vger.kernel.org,
        kernel-team@fb.com
To: Yonghong Song <yhs@fb.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-pf0-f194.google.com ([209.85.192.194]:41522 "EHLO
        mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1754381AbeD3AG3 (ORCPT
        <rfc822;netdev@vger.kernel.org>); Sun, 29 Apr 2018 20:06:29 -0400
Received: by mail-pf0-f194.google.com with SMTP id v63so5400708pfk.8
        for <netdev@vger.kernel.org>; Sun, 29 Apr 2018 17:06:29 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <4524f1c3-a63c-1fb1-ee53-94350e0dc280@fb.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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_* ?