Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

[Jiri Olsa]

On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> > wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > >  wrote:  
> > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > [...]
> > > >>
> > > >> If the purpose of the patch is to give user space visibility into
> > > >> bpf prog load/unload as a notification, then I completely agree that
> > > >> some notification mechanism is necessary.  
> > > 
> > > Yeah, I did only regard it as only that, nothing more. Some means
> > > of timeline and notification that can be kept in a record in user
> > > space and later retrieved e.g. for introspection on what has been
> > > loaded.
> > > 
> > > >> I've started working on such mechanism via perf ring buffer which is
> > > >> the fastest mechanism we have in the kernel so far.
> > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  

I check that discussion and it's related only to bpf program load/unload,
is there any plan to also notify about bpf program attachment?

in the step 2 you described:

  step 2 (future work)
  single event for bpf prog_load with prog_id only.
  Either via perf ring buffer or ftrace or tracepoints or some
  other notification mechanism.

would you see this to be feasible also for bpf prog attachment notification?

thanks,
jirka

Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

[Jiri Olsa]

On Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov wrote:
> On Fri, Oct 05, 2018 at 08:14:09AM +0200, Jiri Olsa wrote:
> > On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> > > On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann 
> > > >  wrote:
> > > > 
> > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > > > >  wrote:  
> > > > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > > > [...]
> > > > > >>
> > > > > >> If the purpose of the patch is to give user space visibility into
> > > > > >> bpf prog load/unload as a notification, then I completely agree 
> > > > > >> that
> > > > > >> some notification mechanism is necessary.  
> > > > > 
> > > > > Yeah, I did only regard it as only that, nothing more. Some means
> > > > > of timeline and notification that can be kept in a record in user
> > > > > space and later retrieved e.g. for introspection on what has been
> > > > > loaded.
> > > > > 
> > > > > >> I've started working on such mechanism via perf ring buffer which 
> > > > > >> is
> > > > > >> the fastest mechanism we have in the kernel so far.
> > > > > >> See long discussion here: 
> > > > > >> https://patchwork.ozlabs.org/patch/971970/  
> > 
> > cool, could you please CC me if there's another version
> > of that patchset?
> 
> will do.

thanks

> 
> > > > > 
> > > > > That one is definitely needed in any case to resolve the kallsyms
> > > > > limitations, and it does have overlap in that in either case we
> > > > > want to look at past BPF programs that have been unloaded in the
> > > > > meantime, so I don't have a strong preference either way, and the
> > > > > former is needed in any case. Though thought was that audit might
> > > > > be an option for those not running profiling daemons 24/7, but
> > > > > presumably bpftool could be extended to record these events as
> > > > > well if we don't want to reuse audit infra.
> > > > 
> > > > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > > > these events.  I do acknowledge that this perf event is relevant,
> > > > especially for catching the kernel symbols (I need that myself), but it
> > > > does not cover my use-case.
> > > > 
> > > > My use-case is to 24/7 collect and keep records in userspace, and have a
> > > > timeline of these notifications, for later retrieval.  The idea is that
> > > > our support engineers can look at these records when troubleshooting
> > > > the system.  And the plan is also to collect these records as part of
> > > > our sosreport tool, which is part of the support case.
> > > 
> > > I don't think you're implying that prog load/unload should be spamming 
> > > dmesg
> > > and auditd not even running...
> > 
> > I think the problem Jesper implied is that in order to collect
> > those logs you'll need perf tool running all the time.. which
> > it's not equipped for yet
> 
> I'm not proposing to run 'perf' binary all the time.
> Setting up perf ring buffer just for these new bpf prog load/unload events
> and epolling it is simple enough to do from any application including auditd.
> selftests/bpf/ do it for bpf output events.

ok, did not think about the possibility to teach auditd talk to perf,
time to get that tool evsel/evlist/rb library ready ;-)

jirka

Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload

[Jiri Olsa]

On Thu, Oct 04, 2018 at 03:10:15PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 04, 2018 at 10:22:31PM +0200, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann  
> > wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
> > > >  wrote:  
> > > >> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:  
> > [...]
> > > >>
> > > >> If the purpose of the patch is to give user space visibility into
> > > >> bpf prog load/unload as a notification, then I completely agree that
> > > >> some notification mechanism is necessary.  
> > > 
> > > Yeah, I did only regard it as only that, nothing more. Some means
> > > of timeline and notification that can be kept in a record in user
> > > space and later retrieved e.g. for introspection on what has been
> > > loaded.
> > > 
> > > >> I've started working on such mechanism via perf ring buffer which is
> > > >> the fastest mechanism we have in the kernel so far.
> > > >> See long discussion here: https://patchwork.ozlabs.org/patch/971970/  

cool, could you please CC me if there's another version
of that patchset?

> > > 
> > > That one is definitely needed in any case to resolve the kallsyms
> > > limitations, and it does have overlap in that in either case we
> > > want to look at past BPF programs that have been unloaded in the
> > > meantime, so I don't have a strong preference either way, and the
> > > former is needed in any case. Though thought was that audit might
> > > be an option for those not running profiling daemons 24/7, but
> > > presumably bpftool could be extended to record these events as
> > > well if we don't want to reuse audit infra.
> > 
> > Yes, exactly, I don't want to run a profiling daemon 24/7 to record
> > these events.  I do acknowledge that this perf event is relevant,
> > especially for catching the kernel symbols (I need that myself), but it
> > does not cover my use-case.
> > 
> > My use-case is to 24/7 collect and keep records in userspace, and have a
> > timeline of these notifications, for later retrieval.  The idea is that
> > our support engineers can look at these records when troubleshooting
> > the system.  And the plan is also to collect these records as part of
> > our sosreport tool, which is part of the support case.
> 
> I don't think you're implying that prog load/unload should be spamming dmesg
> and auditd not even running...

I think the problem Jesper implied is that in order to collect
those logs you'll need perf tool running all the time.. which
it's not equipped for yet

jirka

> Also auditd has to be changed to support retrieving prog related info (like 
> license)
> via sys_bpf system call when it sees prog_id.
> Since it has to change it can just as easily use perf ring buffer
> to receive notifications.
> So we solve notification problem once and all user space tools can use it.
> 

Re: [PATCHv3 3/3] tools bpftool: Display license GPL compatible in prog show/list

[Jiri Olsa]

On Thu, Apr 26, 2018 at 10:49:25PM +0200, Daniel Borkmann wrote:
> On 04/26/2018 10:18 AM, Jiri Olsa wrote:
> [...]
> > v3 of the last patch attached, the branch is also updated
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > Display the license "gpl" string in bpftool prog command, like:
> > 
> >   # bpftool prog list
> >   5: tracepoint  name func  tag 57cd311f2e27366b  gpl
> >   loaded_at Apr 26/09:37  uid 0
> >   xlated 16B  not jited  memlock 4096B
> > 
> >   # bpftool --json --pretty prog show
> >   [{
> >   "id": 5,
> >   "type": "tracepoint",
> >   "name": "func",
> >   "tag": "57cd311f2e27366b",
> >   "gpl_compatible": true,
> >   "loaded_at": "Apr 26/09:37",
> >   "uid": 0,
> >   "bytes_xlated": 16,
> >   "jited": false,
> >   "bytes_memlock": 4096
> >   }
> >   ]
> > 
> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> 
> Ok, v2 from prior two patches and v3 of this one applied to bpf-next. Please
> next time always submit a fresh new series at once, thanks Jiri.

noted, thanks a lot

jirka

[PATCHv3 3/3] tools bpftool: Display license GPL compatible in prog show/list

[Jiri Olsa]

On Thu, Apr 26, 2018 at 09:53:26AM +0200, Daniel Borkmann wrote:
> On 04/26/2018 09:39 AM, Jiri Olsa wrote:
> > On Wed, Apr 25, 2018 at 11:14:30PM +0200, Daniel Borkmann wrote:
> >> On 04/25/2018 11:03 PM, Jakub Kicinski wrote:
> >>> On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> >>>> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info 
> >>>> *info, int fd)
> >>>>  printf("tag ");
> >>>>  fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> >>>>  print_dev_plain(info->ifindex, info->netns_dev, 
> >>>> info->netns_ino);
> >>>> +printf(" license GPL %scompatible", info->gpl_compatible ? "" : 
> >>>> "NON ");
> >>>
> >>> 3 nit picks:
> >>>
> >>> Other "fields" are separated by two spaces between each other:
> >>>
> >>>   4: kprobe  name func_begin  tag 57cd311f2e27366b license GPL compatible
> >>>^^   ^^X
> >>>   loaded_at Apr 25/11:20  uid 0
> >>> ^^
> >>>   xlated 16B  not jited  memlock 4096B
> >>> ^^ ^^
> >>>
> >>> Could you also update the example outputs in the man page:
> >>>
> >>> tools/bpf/bpftool/Documentation/bpftool-prog.rst
> >>>
> >>> Sorry about the bike shedding but I would also vote for:
> >>>
> >>> "[not] GPL compatible"
> >>>
> >>> rather than
> >>>
> >>> "license GPL [NON] compatible"
> >>>
> >>> for brevity..
> >>
> >> While we're at it, can we also squeeze this whole thing a bit? Feels like
> >> huge string wasted for very little information compared to the rest of the
> >> dump. Just append the string "gpl" at the end of the line if 
> >> info->gpl_compatible
> >> is set, otherwise just add nothing. This also allows to naturally grep
> >> for it e.g. `bpftool p | grep gpl` if you need a quick summary.
> > 
> > that's fine with me.. so 'gpl' in here:
> > 
> > 5: tracepoint  name func  tag 57cd311f2e27366b  gpl
> > loaded_at Apr 26/09:37  uid 0
> > xlated 16B  not jited  memlock 4096B
> > 
> > and keeping tyhe whole name in json output:
> > 
> > [{
> > "id": 5,
> > "type": "tracepoint",
> > "name": "func",
> > "tag": "57cd311f2e27366b",
> > "gpl_compatible": true,
> > "loaded_at": "Apr 26/09:37",
> > "uid": 0,
> >     "bytes_xlated": 16,
> > "jited": false,
> > "bytes_memlock": 4096
> > }
> > ]
> > 
> > how about that?
> 
> Sounds good, thanks Jiri!

v3 of the last patch attached, the branch is also updated

thanks,
jirka


---
Display the license "gpl" string in bpftool prog command, like:

  # bpftool prog list
  5: tracepoint  name func  tag 57cd311f2e27366b  gpl
  loaded_at Apr 26/09:37  uid 0
  xlated 16B  not jited  memlock 4096B

  # bpftool --json --pretty prog show
  [{
  "id": 5,
  "type": "tracepoint",
  "name": "func",
  "tag": "57cd311f2e27366b",
  "gpl_compatible": true,
  "loaded_at": "Apr 26/09:37",
  "uid": 0,
  "bytes_xlated": 16,
  "jited": false,
  "bytes_memlock": 4096
  }
  ]

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/Documentation/bpftool-prog.rst | 3 ++-
 tools/bpf/bpftool/prog.c | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst 
b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
index 67ca6c69376c..43d34a5c3ec5 100644
--- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
+++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
@@ -95,7 +95,7 @@ EXAMPLES
 **# bpftool prog show**
 ::
 
-  10: xdp  name some_prog  tag 005a3d2123620c8b
+  10: xdp  name some_prog  tag 005a3d2123620c8b  gpl
loaded_at Sep 29/20:11  uid 0
xlated 528B  jited 370B  memlock 4096B  map_ids 10
 
@@ -108,6 +108,7 @@ EXAMPLES
 &q

Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

[Jiri Olsa]

On Wed, Apr 25, 2018 at 11:14:30PM +0200, Daniel Borkmann wrote:
> On 04/25/2018 11:03 PM, Jakub Kicinski wrote:
> > On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> >> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info 
> >> *info, int fd)
> >>printf("tag ");
> >>fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> >>print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> >> +  printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
> > 
> > 3 nit picks:
> > 
> > Other "fields" are separated by two spaces between each other:
> > 
> >   4: kprobe  name func_begin  tag 57cd311f2e27366b license GPL compatible
> >^^   ^^X
> >   loaded_at Apr 25/11:20  uid 0
> > ^^
> >   xlated 16B  not jited  memlock 4096B
> > ^^ ^^
> > 
> > Could you also update the example outputs in the man page:
> > 
> > tools/bpf/bpftool/Documentation/bpftool-prog.rst
> > 
> > Sorry about the bike shedding but I would also vote for:
> > 
> > "[not] GPL compatible"
> > 
> > rather than
> > 
> > "license GPL [NON] compatible"
> > 
> > for brevity..
> 
> While we're at it, can we also squeeze this whole thing a bit? Feels like
> huge string wasted for very little information compared to the rest of the
> dump. Just append the string "gpl" at the end of the line if 
> info->gpl_compatible
> is set, otherwise just add nothing. This also allows to naturally grep
> for it e.g. `bpftool p | grep gpl` if you need a quick summary.

that's fine with me.. so 'gpl' in here:

5: tracepoint  name func  tag 57cd311f2e27366b  gpl
loaded_at Apr 26/09:37  uid 0
xlated 16B  not jited  memlock 4096B

and keeping tyhe whole name in json output:

[{
"id": 5,
"type": "tracepoint",
"name": "func",
"tag": "57cd311f2e27366b",
"gpl_compatible": true,
"loaded_at": "Apr 26/09:37",
"uid": 0,
"bytes_xlated": 16,
"jited": false,
"bytes_memlock": 4096
}
]

how about that?

thanks,
jirka

Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

[Jiri Olsa]

On Wed, Apr 25, 2018 at 02:03:46PM -0700, Jakub Kicinski wrote:
> On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> > @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info 
> > *info, int fd)
> > printf("tag ");
> > fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> > print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> > +   printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
> 
> 3 nit picks:
> 
> Other "fields" are separated by two spaces between each other:
> 
>   4: kprobe  name func_begin  tag 57cd311f2e27366b license GPL compatible
>^^   ^^X
>   loaded_at Apr 25/11:20  uid 0
> ^^
>   xlated 16B  not jited  memlock 4096B
> ^^ ^^
> 

will change

> Could you also update the example outputs in the man page:
> 
> tools/bpf/bpftool/Documentation/bpftool-prog.rst

oops, I always forget that.. will do

thanks,
jirka

[PATCH 1/3] bpf: Add gpl_compatible flag to struct bpf_prog_info

[Jiri Olsa]

Adding gpl_compatible flag to struct bpf_prog_info
so it can be dumped via bpf_prog_get_info_by_fd and
displayed via bpftool progs dump.

Alexei noticed 4-byte hole in struct bpf_prog_info,
so we put the u32 flags field in there, and we can
keep adding bit fields in there without breaking
user space.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 include/uapi/linux/bpf.h | 1 +
 kernel/bpf/syscall.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e6679393b687..da8801860c7d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1060,6 +1060,7 @@ struct bpf_prog_info {
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
__u32 ifindex;
+   __u32 gpl_compatible:1;
__u64 netns_dev;
__u64 netns_ino;
 } __attribute__((aligned(8)));
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fe23dc5a3ec4..7bb4ff1c770a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1914,6 +1914,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
info.load_time = prog->aux->load_time;
info.created_by_uid = from_kuid_munged(current_user_ns(),
   prog->aux->user->uid);
+   info.gpl_compatible = prog->gpl_compatible;
 
memcpy(info.tag, prog->tag, sizeof(prog->tag));
memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
-- 
2.13.6

[PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list

[Jiri Olsa]

Display the license GPL NON/compatible string in bpftool
prog command, like:

  # bpftool prog list
  3: kprobe  name func_begin  tag 57cd311f2e27366b license GPL NON compatible
  loaded_at Apr 25/11:20  uid 0
  xlated 16B  not jited  memlock 4096B

  # bpftool prog list
  4: kprobe  name func_begin  tag 57cd311f2e27366b license GPL compatible
  loaded_at Apr 25/11:20  uid 0
  xlated 16B  not jited  memlock 4096B

  # bpftool prog show --json
  [{"id":3,"type":"kprobe","name":"func ... ,"gpl_compatible":false,"loade...

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/prog.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 548adb9b7317..b8b4341a1342 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -235,6 +235,8 @@ static void print_prog_json(struct bpf_prog_info *info, int 
fd)
 info->tag[0], info->tag[1], info->tag[2], info->tag[3],
 info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
 
+   jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
+
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
 
if (info->load_time) {
@@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, 
int fd)
printf("tag ");
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
+   printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
printf("\n");
 
if (info->load_time) {
-- 
2.13.6

[PATCH 2/3] tools bpf: Sync bpf.h uapi header

[Jiri Olsa]

Syncing the bpf.h uapi header with tools.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/include/uapi/linux/bpf.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e6679393b687..da8801860c7d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -1060,6 +1060,7 @@ struct bpf_prog_info {
__aligned_u64 map_ids;
char name[BPF_OBJ_NAME_LEN];
__u32 ifindex;
+   __u32 gpl_compatible:1;
__u64 netns_dev;
__u64 netns_ino;
 } __attribute__((aligned(8)));
-- 
2.13.6

[PATCHv2 0/3] bpf: Store/dump license string for loaded program

[Jiri Olsa]

hi,
sending the change to store and dump the license
info for loaded BPF programs. It's important for
us get the license info, when investigating on
screwed up machine.

v2 changes:
  - dumping only the GPL compatible bool, without
storing the whole license string

Adding change to bpftool to dump the license GPL
compatible info via:

  # bpftool prog list
  3: kprobe  name func_begin  tag 57cd311f2e27366b license GPL NON compatible
  loaded_at Apr 25/11:20  uid 0
  xlated 16B  not jited  memlock 4096B

  # bpftool prog list
  4: kprobe  name func_begin  tag 57cd311f2e27366b license GPL compatible
  loaded_at Apr 25/11:20  uid 0
  xlated 16B  not jited  memlock 4096B

  # bpftool prog show --json
  [{"id":3,"type":"kprobe","name":"func ... ,"gpl_compatible":false,"loade...

Also available at:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/license

thanks,
jirka


---
Jiri Olsa (3):
  bpf: Add gpl_compatible flag to struct bpf_prog_info
  tools bpf: Sync bpf.h uapi header
  tools bpftool: Display license GPL compatible in prog show/list

 include/uapi/linux/bpf.h   | 1 +
 kernel/bpf/syscall.c   | 1 +
 tools/bpf/bpftool/prog.c   | 3 +++
 tools/include/uapi/linux/bpf.h | 1 +
 4 files changed, 6 insertions(+)

Re: [PATCH 0/3] bpf: Store/dump license string for loaded program

[Jiri Olsa]

On Wed, Apr 25, 2018 at 10:15:29AM -0600, Alexei Starovoitov wrote:
> On Wed, Apr 25, 2018 at 12:17:13PM +0200, Jiri Olsa wrote:
> > On Mon, Apr 23, 2018 at 02:11:36PM -0600, Alexei Starovoitov wrote:
> > > On Mon, Apr 23, 2018 at 08:59:24AM +0200, Jiri Olsa wrote:
> > > > hi,
> > > > sending the change to store and dump the license
> > > > info for loaded BPF programs. It's important for
> > > > us get the license info, when investigating on
> > > > screwed up machine.
> > > 
> > > hmm. boolean flag whether bpf prog is gpl or not
> > > is already exposed via bpf_prog_info.
> > 
> > hum, I can't see that (on bpf-next/master)
> > would the attached change be ok with you?
> > 
> > jirka
> > 
> > 
> > ---
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index e6679393b687..2ce9c9d41c2b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1062,6 +1062,7 @@ struct bpf_prog_info {
> > __u32 ifindex;
> > __u64 netns_dev;
> > __u64 netns_ino;
> > +   __u16 gpl_compatible:1;
> >  } __attribute__((aligned(8)));
> 
> ahh. I swear there were patches to add it and I thought we accepted them.
> Also just noticed that commit 675fc275a3a2d added 4-byte hole in there.
> So I'm thinking we can fill the hole with 
>   __u32 ifindex;
> + __u32 gpl_compatible:1;
>   __u64 netns_dev;
>   __u64 netns_ino;
> 
> and keep adding bit fields in there without breaking user space.
> Such patch would need to go to bpf tree.
> 

ok, will post v2 with that

thanks,
jirka

Re: [PATCH 0/3] bpf: Store/dump license string for loaded program

[Jiri Olsa]

On Mon, Apr 23, 2018 at 02:11:36PM -0600, Alexei Starovoitov wrote:
> On Mon, Apr 23, 2018 at 08:59:24AM +0200, Jiri Olsa wrote:
> > hi,
> > sending the change to store and dump the license
> > info for loaded BPF programs. It's important for
> > us get the license info, when investigating on
> > screwed up machine.
> 
> hmm. boolean flag whether bpf prog is gpl or not
> is already exposed via bpf_prog_info.

hum, I can't see that (on bpf-next/master)
would the attached change be ok with you?

jirka


---
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e6679393b687..2ce9c9d41c2b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -1062,6 +1062,7 @@ struct bpf_prog_info {
__u32 ifindex;
__u64 netns_dev;
__u64 netns_ino;
+   __u16 gpl_compatible:1;
 } __attribute__((aligned(8)));
 
 struct bpf_map_info {
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fe23dc5a3ec4..7bb4ff1c770a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1914,6 +1914,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
info.load_time = prog->aux->load_time;
info.created_by_uid = from_kuid_munged(current_user_ns(),
   prog->aux->user->uid);
+   info.gpl_compatible = prog->gpl_compatible;
 
memcpy(info.tag, prog->tag, sizeof(prog->tag));
memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));

[PATCH 0/3] bpf: Store/dump license string for loaded program

[Jiri Olsa]

hi,
sending the change to store and dump the license
info for loaded BPF programs. It's important for
us get the license info, when investigating on
screwed up machine.

Adding change to bpftool to dump the license via:

  # bpftool prog list
  1: kprobe  name func_begin  tag 59bef35a42fda602 license GPL
  loaded_at Apr 22/15:46  uid 0
  xlated 272B  not jited  memlock 4096B  map_ids 1

  # bpftool prog show --json
  [{"id":1,"type":"kprobe","name":"fun ... ,"license":"GPL", ... ]

Also available at:
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/license

thanks,
jirka


---
Jiri Olsa (3):
  bpf: Store license string for loaded program
  tools bpf: Sync bpf.h uapi header
  tools bpftool: Display license in prog show/list

 include/linux/bpf.h| 1 +
 include/uapi/linux/bpf.h   | 3 +++
 kernel/bpf/core.c  | 1 +
 kernel/bpf/syscall.c   | 7 ++-
 tools/bpf/bpftool/prog.c   | 4 
 tools/include/uapi/linux/bpf.h | 4 
 6 files changed, 19 insertions(+), 1 deletion(-)

[PATCH 1/3] bpf: Store license string for loaded program

[Jiri Olsa]

Storing the license string provided loaded program,
so it can be provided back when dumping the loaded
programs info (added in following change).

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 include/linux/bpf.h  | 1 +
 include/uapi/linux/bpf.h | 3 +++
 kernel/bpf/core.c| 1 +
 kernel/bpf/syscall.c | 7 ++-
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index ee5275e7d4df..8530b791c1b0 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -274,6 +274,7 @@ struct bpf_prog_aux {
struct user_struct *user;
u64 load_time; /* ns since boottime */
char name[BPF_OBJ_NAME_LEN];
+   char *license;
 #ifdef CONFIG_SECURITY
void *security;
 #endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c8383a289f7b..b7298ee177e7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -239,6 +239,8 @@ enum bpf_attach_type {
 
 #define BPF_OBJ_NAME_LEN 16U
 
+#define BPF_OBJ_LICENSE_LEN 128U
+
 /* Flags for accessing BPF object */
 #define BPF_F_RDONLY   (1U << 3)
 #define BPF_F_WRONLY   (1U << 4)
@@ -1039,6 +1041,7 @@ struct bpf_prog_info {
__u32 ifindex;
__u64 netns_dev;
__u64 netns_ino;
+   char license[BPF_OBJ_LICENSE_LEN];
 } __attribute__((aligned(8)));
 
 struct bpf_map_info {
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index d315b393abdd..05352a928917 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -142,6 +142,7 @@ struct bpf_prog *bpf_prog_realloc(struct bpf_prog *fp_old, 
unsigned int size,
 
 void __bpf_prog_free(struct bpf_prog *fp)
 {
+   kfree(fp->aux->license);
kfree(fp->aux);
vfree(fp);
 }
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index fe23dc5a3ec4..a876af93147f 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1265,7 +1265,7 @@ static int bpf_prog_load(union bpf_attr *attr)
enum bpf_prog_type type = attr->prog_type;
struct bpf_prog *prog;
int err;
-   char license[128];
+   char license[BPF_OBJ_LICENSE_LEN];
bool is_gpl;
 
if (CHECK_ATTR(BPF_PROG_LOAD))
@@ -1345,6 +1345,10 @@ static int bpf_prog_load(union bpf_attr *attr)
if (err)
goto free_prog;
 
+   prog->aux->license = kstrdup(license, GFP_KERNEL);
+   if (!prog->aux->license)
+   goto free_prog;
+
/* run eBPF verifier */
err = bpf_check(, attr);
if (err < 0)
@@ -1917,6 +1921,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
 
memcpy(info.tag, prog->tag, sizeof(prog->tag));
memcpy(info.name, prog->aux->name, sizeof(prog->aux->name));
+   strncpy(info.license, prog->aux->license, BPF_OBJ_LICENSE_LEN);
 
ulen = info.nr_map_ids;
info.nr_map_ids = prog->aux->used_map_cnt;
-- 
2.13.6

[PATCH 3/3] tools bpftool: Display license in prog show/list

[Jiri Olsa]

Display the license string in bpftool prog command, like:

  # bpftool prog list
  1: kprobe  name func_begin  tag 59bef35a42fda602 license GPL
  loaded_at Apr 22/15:46  uid 0
  xlated 272B  not jited  memlock 4096B  map_ids 1

  # bpftool prog show --json
  [{"id":1,"type":"kprobe","name":"fun ... ,"license":"GPL", ... ]

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/prog.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index 548adb9b7317..ea5ede899cf7 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -235,6 +235,8 @@ static void print_prog_json(struct bpf_prog_info *info, int 
fd)
 info->tag[0], info->tag[1], info->tag[2], info->tag[3],
 info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
 
+   jsonw_string_field(json_wtr, "license", info->license);
+
print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
 
if (info->load_time) {
@@ -295,8 +297,10 @@ static void print_prog_plain(struct bpf_prog_info *info, 
int fd)
printf("tag ");
fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
+   printf(" license %s", info->license);
printf("\n");
 
+
if (info->load_time) {
char buf[32];
 
-- 
2.13.6

[PATCH 2/3] tools bpf: Sync bpf.h uapi header

[Jiri Olsa]

Syncing the bpf.h uapi header with tools.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/include/uapi/linux/bpf.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 7f7fbb9d0253..b7298ee177e7 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -239,6 +239,8 @@ enum bpf_attach_type {
 
 #define BPF_OBJ_NAME_LEN 16U
 
+#define BPF_OBJ_LICENSE_LEN 128U
+
 /* Flags for accessing BPF object */
 #define BPF_F_RDONLY   (1U << 3)
 #define BPF_F_WRONLY   (1U << 4)
@@ -884,6 +886,7 @@ enum bpf_func_id {
 /* BPF_FUNC_skb_set_tunnel_key flags. */
 #define BPF_F_ZERO_CSUM_TX (1ULL << 1)
 #define BPF_F_DONT_FRAGMENT(1ULL << 2)
+#define BPF_F_SEQ_NUMBER   (1ULL << 3)
 
 /* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
  * BPF_FUNC_perf_event_read_value flags.
@@ -1038,6 +1041,7 @@ struct bpf_prog_info {
__u32 ifindex;
__u64 netns_dev;
__u64 netns_ino;
+   char license[BPF_OBJ_LICENSE_LEN];
 } __attribute__((aligned(8)));
 
 struct bpf_map_info {
-- 
2.13.6

Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh

[Jiri Olsa]

On Fri, Apr 06, 2018 at 09:59:59AM +0900, Masahiro Yamada wrote:
> 2018-04-06 3:59 GMT+09:00 Jiri Olsa <jo...@redhat.com>:
> > On Fri, Apr 06, 2018 at 12:50:00AM +0900, Masahiro Yamada wrote:
> >> 2018-04-06 0:16 GMT+09:00 Jiri Olsa <jo...@kernel.org>:
> >> > There's no need to pass LD* arguments to link-vmlinux.sh,
> >> > because they are passed as variables. The only argument
> >> > the link-vmlinux.sh supports is the 'clean' argument.
> >> >
> >> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> >> > ---
> >>
> >> Wrong.
> >>
> >> $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> >> exist here so that any change in them
> >> invokes scripts/linkk-vmlinux.sh
> >
> > sry, I can't see that.. but it's just a side fix,
> > which is actually not needed for the rest
> >
> > I'll check on more and address this separately
> 
> 
> The link command is recorded in .vmlinux.cmd

i see.. just for the sake command line,

thanks for explanation
jirka

Re: [RFC 0/9] bpf: Add buildid check support

[Jiri Olsa]

On Thu, Apr 05, 2018 at 06:37:23PM -0700, Alexei Starovoitov wrote:
> On Thu, Apr 05, 2018 at 05:16:36PM +0200, Jiri Olsa wrote:
> > hi,
> > eBPF programs loaded for kprobes are allowed to read kernel
> > internal structures. We check the provided kernel version
> > to ensure that the program is loaded for the proper kernel. 
> > 
> > The problem is that the version check is not enough, because
> > it only follows the version setup from kernel's Makefile.
> > However, the internal kernel structures change based on the
> > .config data, so in practise we have different kernels with
> > same version.
> > 
> > The eBPF kprobe program thus then get loaded for different
> > kernel than it's been built for, get wrong data (silently)
> > and provide misleading output.
> > 
> > This patchset implements additional check in eBPF loading code
> > on provided build ID (from kernel's elf image, .notes section
> > GNU build ID) to ensure we load the eBPF program on correct
> > kernel.
> > 
> > Also available in here (based on bpf-next/master):
> >   https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   bpf/checksum
> > 
> > This patchset consists of several changes:
> > 
> > - adding CONFIG_BUILDID_H option that instructs the build
> >   to generate uapi header file with build ID data, that
> >   will be included by eBPF program
> > 
> > - adding CONFIG_BPF_BUILDID_CHECK option and new bpf_attr
> >   field to allow build ID checking when loading the eBPF
> >   program
> > 
> > - changing libbpf to read and pass build ID to the kernel
> > 
> > - several small side fixes
> > 
> > - example perf eBPF code in bpf-samples/bpf-stdout-example.c
> >   to show the build ID support/usage.
> > 
> > # perf record -vv  -e ./bpf-samples/bpf-stdout-example.c kill 2>&1 | 
> > grep buildid
> > libbpf: section(7) buildid, size 21, link 0, flags 3, type=1
> > libbpf: kernel buildid of ./bpf-samples/bpf-stdout-example.c is: 
> > 6e25edeb408513184e2753bebad25d42314501a0
> > 
> >   The buildid is provided the same way we provide kernel
> >   version, in a special "buildid" section:
> > 
> > # cat ./bpf-samples/bpf-stdout-example.c
> > ...
> > #include 
> > 
> > char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA;
> > ...
> > 
> >   where LINUX_BUILDID_DATA is defined in the generated buildid.h.
> > 
> > please note it's an RFC ;-) any comments and suggestions are welcome
> 
> I think this is overkill.
> 
> We're very heavy users of kprobe+bpf. It's used for lots
> of different cases and usage is constantly growing,
> but I haven't seen a single case of :
> 
> > The eBPF kprobe program thus then get loaded for different
> > kernel than it's been built for, get wrong data (silently)
> > and provide misleading output.
> 
> but I saw plenty of the opposite. People pre-compile the program
> and hack kernel version when they load, since they know in advance
> that kprobe+bpf doesn't use any kernel specific things.
> The existing kernel version check for kprobe+bpf is already annoying
> to them.

perhaps verifier could detect this (via bpf_probe_read usage) and disable
the version check automaticaly for such program?

and in the same way force the version check (or buildid when enabled)
once the bpf_probe_read is detected

thanks,
jirka

Re: [PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh

[Jiri Olsa]

On Fri, Apr 06, 2018 at 12:50:00AM +0900, Masahiro Yamada wrote:
> 2018-04-06 0:16 GMT+09:00 Jiri Olsa <jo...@kernel.org>:
> > There's no need to pass LD* arguments to link-vmlinux.sh,
> > because they are passed as variables. The only argument
> > the link-vmlinux.sh supports is the 'clean' argument.
> >
> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> > ---
> 
> Wrong.
> 
> $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux)
> exist here so that any change in them
> invokes scripts/linkk-vmlinux.sh

sry, I can't see that.. but it's just a side fix,
which is actually not needed for the rest

I'll check on more and address this separately

thanks,
jirka

> 
> 
> 
> 
> >  Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index d3300e46f925..a65a3919c6ad 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard 
> > $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
> >
> >  # Final link of vmlinux with optional arch pass after final link
> >  cmd_link-vmlinux = \
> > -   $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
> > +   $(CONFIG_SHELL) $< ;   \
> > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> >
> >  vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
> 
> 
> 
> 
> 
> 
> -- 
> Best Regards
> Masahiro Yamada

[RFC 0/9] bpf: Add buildid check support

[Jiri Olsa]

hi,
eBPF programs loaded for kprobes are allowed to read kernel
internal structures. We check the provided kernel version
to ensure that the program is loaded for the proper kernel. 

The problem is that the version check is not enough, because
it only follows the version setup from kernel's Makefile.
However, the internal kernel structures change based on the
.config data, so in practise we have different kernels with
same version.

The eBPF kprobe program thus then get loaded for different
kernel than it's been built for, get wrong data (silently)
and provide misleading output.

This patchset implements additional check in eBPF loading code
on provided build ID (from kernel's elf image, .notes section
GNU build ID) to ensure we load the eBPF program on correct
kernel.

Also available in here (based on bpf-next/master):
  https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
  bpf/checksum

This patchset consists of several changes:

- adding CONFIG_BUILDID_H option that instructs the build
  to generate uapi header file with build ID data, that
  will be included by eBPF program

- adding CONFIG_BPF_BUILDID_CHECK option and new bpf_attr
  field to allow build ID checking when loading the eBPF
  program

- changing libbpf to read and pass build ID to the kernel

- several small side fixes

- example perf eBPF code in bpf-samples/bpf-stdout-example.c
  to show the build ID support/usage.

# perf record -vv  -e ./bpf-samples/bpf-stdout-example.c kill 2>&1 | grep 
buildid
libbpf: section(7) buildid, size 21, link 0, flags 3, type=1
libbpf: kernel buildid of ./bpf-samples/bpf-stdout-example.c is: 
6e25edeb408513184e2753bebad25d42314501a0

  The buildid is provided the same way we provide kernel
  version, in a special "buildid" section:

# cat ./bpf-samples/bpf-stdout-example.c
...
#include 

char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA;
...

  where LINUX_BUILDID_DATA is defined in the generated buildid.h.

please note it's an RFC ;-) any comments and suggestions are welcome

thanks,
jirka


---
Jiri Olsa (9):
  perf tools: Make read_build_id function public
  perf tools: Add fetch_kernel_buildid function
  kbuild: Do not pass arguments to link-vmlinux.sh
  kbuild: Add filechk2 function
  bpf: Add CONFIG_BUILDID_H option
  bpf: Add CONFIG_BPF_BUILDID_CHECK option
  libbpf: Synchronize uapi bpf.h header
  libbpf: Add support to attach buildid to program load
  perf tools: The buildid usage in example eBPF program

 Makefile| 14 +-
 include/uapi/linux/bpf.h|  2 ++
 init/Kconfig| 12 
 kernel/bpf/syscall.c| 84 
+++-
 scripts/Kbuild.include  | 24 
 scripts/Makefile|  1 +
 scripts/extract-buildid.c   | 42 
++
 tools/bpf/bpftool/Makefile  |  5 -
 tools/include/uapi/linux/bpf.h  |  3 +++
 tools/lib/bpf/bpf.c |  6 --
 tools/lib/bpf/bpf.h |  5 +++--
 tools/lib/bpf/libbpf.c  | 46 
--
 tools/perf/bpf-samples/bpf-stdout-example.c | 42 
++
 tools/perf/tests/bpf.c  |  9 -
 tools/perf/util/symbol-minimal.c| 50 
++
 tools/perf/util/util.c  | 66 
++
 tools/perf/util/util.h  |  6 ++
 17 files changed, 355 insertions(+), 62 deletions(-)
 create mode 100644 scripts/extract-buildid.c
 create mode 100644 tools/perf/bpf-samples/bpf-stdout-example.c

[PATCH 1/9] perf tools: Make read_build_id function public

[Jiri Olsa]

And renaming it into parse_notes_buildid to be more
precise and usable in following patches.

Link: http://lkml.kernel.org/n/tip-v1mz76rkdxfnbfz2v05fu...@git.kernel.org
Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/perf/util/symbol-minimal.c | 50 ++--
 tools/perf/util/util.c   | 48 ++
 tools/perf/util/util.h   |  4 
 3 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/tools/perf/util/symbol-minimal.c b/tools/perf/util/symbol-minimal.c
index ff48d0d49584..bd281d3dc508 100644
--- a/tools/perf/util/symbol-minimal.c
+++ b/tools/perf/util/symbol-minimal.c
@@ -24,53 +24,6 @@ static bool check_need_swap(int file_endian)
return host_endian != file_endian;
 }
 
-#define NOTE_ALIGN(sz) (((sz) + 3) & ~3)
-
-#define NT_GNU_BUILD_ID3
-
-static int read_build_id(void *note_data, size_t note_len, void *bf,
-size_t size, bool need_swap)
-{
-   struct {
-   u32 n_namesz;
-   u32 n_descsz;
-   u32 n_type;
-   } *nhdr;
-   void *ptr;
-
-   ptr = note_data;
-   while (ptr < (note_data + note_len)) {
-   const char *name;
-   size_t namesz, descsz;
-
-   nhdr = ptr;
-   if (need_swap) {
-   nhdr->n_namesz = bswap_32(nhdr->n_namesz);
-   nhdr->n_descsz = bswap_32(nhdr->n_descsz);
-   nhdr->n_type = bswap_32(nhdr->n_type);
-   }
-
-   namesz = NOTE_ALIGN(nhdr->n_namesz);
-   descsz = NOTE_ALIGN(nhdr->n_descsz);
-
-   ptr += sizeof(*nhdr);
-   name = ptr;
-   ptr += namesz;
-   if (nhdr->n_type == NT_GNU_BUILD_ID &&
-   nhdr->n_namesz == sizeof("GNU")) {
-   if (memcmp(name, "GNU", sizeof("GNU")) == 0) {
-   size_t sz = min(size, descsz);
-   memcpy(bf, ptr, sz);
-   memset(bf + sz, 0, size - sz);
-   return 0;
-   }
-   }
-   ptr += descsz;
-   }
-
-   return -1;
-}
-
 int filename__read_debuglink(const char *filename __maybe_unused,
 char *debuglink __maybe_unused,
 size_t size __maybe_unused)
@@ -153,7 +106,8 @@ int filename__read_build_id(const char *filename, void *bf, 
size_t size)
if (fread(buf, buf_size, 1, fp) != 1)
goto out_free;
 
-   ret = read_build_id(buf, buf_size, bf, size, need_swap);
+   ret = parse_notes_buildid(buf, buf_size, bf, size,
+ need_swap);
if (ret == 0)
ret = size;
break;
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 1019bbc5dbd8..41fc61d941ef 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -447,6 +447,54 @@ fetch_kernel_version(unsigned int *puint, char *str,
return 0;
 }
 
+#define NOTE_ALIGN(sz) (((sz) + 3) & ~3)
+
+#define NT_GNU_BUILD_ID3
+
+int parse_notes_buildid(void *note_data, size_t note_len, void *bf,
+   size_t size, bool need_swap)
+{
+   struct {
+   u32 n_namesz;
+   u32 n_descsz;
+   u32 n_type;
+   } *nhdr;
+   void *ptr;
+
+   ptr = note_data;
+   while (ptr < (note_data + note_len)) {
+   const char *name;
+   size_t namesz, descsz;
+
+   nhdr = ptr;
+   if (need_swap) {
+   nhdr->n_namesz = bswap_32(nhdr->n_namesz);
+   nhdr->n_descsz = bswap_32(nhdr->n_descsz);
+   nhdr->n_type = bswap_32(nhdr->n_type);
+   }
+
+   namesz = NOTE_ALIGN(nhdr->n_namesz);
+   descsz = NOTE_ALIGN(nhdr->n_descsz);
+
+   ptr += sizeof(*nhdr);
+   name = ptr;
+   ptr += namesz;
+   if (nhdr->n_type == NT_GNU_BUILD_ID &&
+   nhdr->n_namesz == sizeof("GNU")) {
+   if (memcmp(name, "GNU", sizeof("GNU")) == 0) {
+   size_t sz = min(size, descsz);
+
+   memcpy(bf, ptr, sz);
+   memset(bf + sz, 0, size - sz);
+   return 0;
+   }
+   }
+   ptr += descsz;
+   }
+
+   return -1;
+}
+
 const char *perf_tip(const char *dirpath)
 {
struct strlist *tips;
diff --git a/tools

[PATCH 2/9] perf tools: Add fetch_kernel_buildid function

[Jiri Olsa]

Adding fetch_kernel_buildid helper function to
retrieve build id from running kernel. It will
be used in following patches.

Link: http://lkml.kernel.org/n/tip-at98orsncas8v2ito61u3...@git.kernel.org
Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/perf/util/util.c | 18 ++
 tools/perf/util/util.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index 41fc61d941ef..26f93866bc02 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -495,6 +495,24 @@ int parse_notes_buildid(void *note_data, size_t note_len, 
void *bf,
return -1;
 }
 
+int fetch_kernel_buildid(char *buildid, int size)
+{
+   char path[PATH_MAX], *buf;
+   size_t len;
+   int err;
+
+   scnprintf(path, PATH_MAX, "%s/kernel/notes",
+ sysfs__mountpoint());
+
+   if (filename__read_str(path, , ))
+   return -EINVAL;
+
+   err = parse_notes_buildid(buf, len, buildid, size, false);
+
+   free(buf);
+   return err;
+}
+
 const char *perf_tip(const char *dirpath)
 {
struct strlist *tips;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 27106548396b..1ccaa957e3ab 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -48,6 +48,8 @@ extern int cacheline_size;
 int fetch_kernel_version(unsigned int *puint,
 char *str, size_t str_sz);
 
+int fetch_kernel_buildid(char *buildid, int size);
+
 int parse_notes_buildid(void *note_data, size_t note_len, void *bf,
size_t size, bool need_swap);
 
-- 
2.13.6

[PATCH 4/9] kbuild: Add filechk2 function

[Jiri Olsa]

Adding filechk2 function  that has the same semantics
as filechk, but it takes the target file from the 2nd
argument instead of from the '$@' as in the filechk
function.

This function is needed when we can't have separate
target for the file, like in the following patch.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 scripts/Kbuild.include | 24 
 1 file changed, 24 insertions(+)

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 065324a8046f..9775ce2771d4 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -67,6 +67,30 @@ define filechk
fi
 endef
 
+# filechk2 is used to check if the content of a generated file is updated.
+# It follows the same logic as the filechk except instead of the $@ target
+# variable, the checked file is passed in 2nd argument.
+#
+# Sample usage:
+# define filechk_sample
+#  echo $KERNELRELEASE
+# endef
+# version.h : Makefile
+#  $(call filechk2,sample,version.h)
+# endef
+define filechk2
+   $(Q)set -e; \
+   $(kecho) '  CHK $(2)';  \
+   mkdir -p $(dir $(2));   \
+   $(filechk_$(1)) < $< > $(2).tmp;\
+   if [ -r $(2) ] && cmp -s $(2) $(2).tmp; then\
+   rm -f $(2).tmp; \
+   else\
+   $(kecho) '  UPD $(2)';  \
+   mv -f $(2).tmp $(2);\
+   fi
+endef
+
 ##
 # gcc support functions
 # See documentation in Documentation/kbuild/makefiles.txt
-- 
2.13.6

[PATCH 3/9] kbuild: Do not pass arguments to link-vmlinux.sh

[Jiri Olsa]

There's no need to pass LD* arguments to link-vmlinux.sh,
because they are passed as variables. The only argument
the link-vmlinux.sh supports is the 'clean' argument.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index d3300e46f925..a65a3919c6ad 100644
--- a/Makefile
+++ b/Makefile
@@ -1027,7 +1027,7 @@ ARCH_POSTLINK := $(wildcard 
$(srctree)/arch/$(SRCARCH)/Makefile.postlink)
 
 # Final link of vmlinux with optional arch pass after final link
 cmd_link-vmlinux = \
-   $(CONFIG_SHELL) $< $(LD) $(LDFLAGS) $(LDFLAGS_vmlinux) ;\
+   $(CONFIG_SHELL) $< ;   \
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
 vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
-- 
2.13.6

[PATCH 5/9] bpf: Add CONFIG_BUILDID_H option

[Jiri Olsa]

Adding CONFIG_BUILDID_H option that forces build to generate
file with GNU build id value:

  include/linux/buildid.h

It contains following macros:

  #define LINUX_BUILDID_DATA "\x6c\x41\x0f\xea\xa9\x5d ...
  #define LINUX_BUILDID_SIZE 20

Those macros will be used in following patches to identify
kernel in more precise way when loading eBPF program that
can touch kernel internal structures.

There's new build output for the check and update
of the buildid.h:

$ make
...
LD  vmlinux.o
MODPOST vmlinux.o
KSYM.tmp_kallsyms1.o
KSYM.tmp_kallsyms2.o
LD  vmlinux
SORTEX  vmlinux
SYSMAP  System.map
CHK include/generated/uapi/linux/buildid.h
UPD include/generated/uapi/linux/buildid.h
...

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 Makefile  | 12 
 init/Kconfig  |  3 +++
 scripts/Makefile  |  1 +
 scripts/extract-buildid.c | 42 ++
 4 files changed, 58 insertions(+)
 create mode 100644 scripts/extract-buildid.c

diff --git a/Makefile b/Makefile
index a65a3919c6ad..92b04d8f08bc 100644
--- a/Makefile
+++ b/Makefile
@@ -1023,6 +1023,15 @@ endif
 include/generated/autoksyms.h: FORCE
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/adjust_autoksyms.sh true
 
+ifdef CONFIG_BUILDID_H
+buildid_h := include/linux/buildid.h
+
+define filechk_buildid.h
+   buildid=`readelf -n $@ | grep 'Build ID' | sed -e 's/^.*Build ID: 
\(.*\)$$/\1/'`; \
+   scripts/extract-buildid $$buildid
+endef
+endif
+
 ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
 
 # Final link of vmlinux with optional arch pass after final link
@@ -1032,6 +1041,9 @@ cmd_link-vmlinux =
 \
 
 vmlinux: scripts/link-vmlinux.sh vmlinux_prereq $(vmlinux-deps) FORCE
+$(call if_changed,link-vmlinux)
+ifdef CONFIG_BUILDID_H
+   +$(call filechk2,buildid.h,$(buildid_h))
+endif
 
 # Build samples along the rest of the kernel
 ifdef CONFIG_SAMPLES
diff --git a/init/Kconfig b/init/Kconfig
index 2852692d7c9c..572df24dda9b 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1386,6 +1386,9 @@ config KALLSYMS_BASE_RELATIVE
 
 # end of the "standard kernel features (expert users)" menu
 
+config BUILDID_H
+   bool
+
 # syscall, maps, verifier
 config BPF_SYSCALL
bool "Enable bpf() system call"
diff --git a/scripts/Makefile b/scripts/Makefile
index 25ab143cbe14..fa34eaed6c29 100644
--- a/scripts/Makefile
+++ b/scripts/Makefile
@@ -19,6 +19,7 @@ hostprogs-$(CONFIG_ASN1)   += asn1_compiler
 hostprogs-$(CONFIG_MODULE_SIG)  += sign-file
 hostprogs-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += extract-cert
 hostprogs-$(CONFIG_SYSTEM_EXTRA_CERTIFICATE) += insert-sys-cert
+hostprogs-$(CONFIG_BUILDID_H)   += extract-buildid
 
 HOSTCFLAGS_sortextable.o = -I$(srctree)/tools/include
 HOSTCFLAGS_asn1_compiler.o = -I$(srctree)/include
diff --git a/scripts/extract-buildid.c b/scripts/extract-buildid.c
new file mode 100644
index ..a116723da3ad
--- /dev/null
+++ b/scripts/extract-buildid.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Formats buildid into following macros:
+ *
+ * #define LINUX_BUILDID_DATA "\x6c\x41\x0f\xea\xa9\x5d\x46 ...
+ * #define LINUX_BUILDID_SIZE 20
+ *
+ */
+#include 
+#include 
+
+int main(int argc, char **argv)
+{
+   char *id;
+   int len, i;
+
+   if (argc != 2) {
+   fprintf(stderr, "usage: %s buildid\n", argv[0]);
+   return -1;
+   }
+
+   id  = argv[1];
+   len = strlen(id);
+
+   printf("#ifndef _LINUX_BUILDID_H\n");
+   printf("#define _LINUX_BUILDID_H\n");
+   printf("\n");
+
+   printf("#define LINUX_BUILDID_DATA \"");
+
+   for (i = 0; i < len; i += 2)
+   printf("\\x%c%c", id[i], id[i + 1]);
+
+   printf("\"\n");
+
+   printf("#define LINUX_BUILDID_SIZE %u\n", len / 2);
+
+   printf("\n");
+   printf("#endif /* _LINUX_BUILDID_H */\n");
+   return 0;
+}
-- 
2.13.6

[PATCH 7/9] libbpf: Synchronize uapi bpf.h header

[Jiri Olsa]

Synchronize include/uapi/linux/bpf.h with tools version.

Link: http://lkml.kernel.org/n/tip-gaja0nnet6oku657642nx...@git.kernel.org
Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/include/uapi/linux/bpf.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 9d07465023a2..17d8d330e6c7 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -308,6 +308,8 @@ union bpf_attr {
 * (context accesses, allowed helpers, etc).
 */
__u32   expected_attach_type;
+   /* Checked when prog_type=kprobe and CONFIG_BPF_BUILDID_CHECK. 
*/
+   __aligned_u64   kern_buildid;
};
 
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -864,6 +866,7 @@ enum bpf_func_id {
 /* BPF_FUNC_skb_set_tunnel_key flags. */
 #define BPF_F_ZERO_CSUM_TX (1ULL << 1)
 #define BPF_F_DONT_FRAGMENT(1ULL << 2)
+#define BPF_F_SEQ_NUMBER   (1ULL << 3)
 
 /* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
  * BPF_FUNC_perf_event_read_value flags.
-- 
2.13.6

[PATCH 6/9] bpf: Add CONFIG_BPF_BUILDID_CHECK option

[Jiri Olsa]

Adding CONFIG_BPF_BUILDID_CHECK option that forces kernel
to check on provided build id when loading eBPF program.
If the build id does not match the one in kernel the program
fails to load.

Adding new field into struct bpf_attr. The kern_buildid
points to the user memory that contains the buildid.

Kernel expose the notes section via sysfs, but there's
currently no other use for kernel's buildid, so I needed
to add new __init buildid_init function to parse it out.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 include/uapi/linux/bpf.h |  2 ++
 init/Kconfig |  9 ++
 kernel/bpf/syscall.c | 84 +++-
 3 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index c5ec89732a8d..17d8d330e6c7 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -308,6 +308,8 @@ union bpf_attr {
 * (context accesses, allowed helpers, etc).
 */
__u32   expected_attach_type;
+   /* Checked when prog_type=kprobe and CONFIG_BPF_BUILDID_CHECK. 
*/
+   __aligned_u64   kern_buildid;
};
 
struct { /* anonymous struct used by BPF_OBJ_* commands */
diff --git a/init/Kconfig b/init/Kconfig
index 572df24dda9b..6d32220de7e0 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1406,6 +1406,15 @@ config BPF_JIT_ALWAYS_ON
  Enables BPF JIT and removes BPF interpreter to avoid
  speculative execution of BPF instructions by the interpreter
 
+config BPF_BUILDID_CHECK
+   bool "Check buildid for kprobe and tracepoint programs"
+   depends on BPF_SYSCALL
+   select BUILDID_H
+   default n
+   help
+ Enables BPF program load code to check on kernel Build ID
+ for kprobe programs.
+
 config USERFAULTFD
bool "Enable userfaultfd() system call"
select ANON_INODES
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 0244973ee544..d0b3bc0bd9e6 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1239,7 +1239,85 @@ static int bpf_prog_attach_check_attach_type(const 
struct bpf_prog *prog,
 }
 
 /* last field in 'union bpf_attr' used by this command */
-#defineBPF_PROG_LOAD_LAST_FIELD expected_attach_type
+#defineBPF_PROG_LOAD_LAST_FIELD kern_buildid
+
+#ifdef CONFIG_BPF_BUILDID_CHECK
+
+static struct {
+   const char *ptr;
+   int size;
+} buildid;
+
+#define BUILDID_MAX 40
+
+static int __check_buildid(union bpf_attr *attr)
+{
+   char id[buildid.size];
+
+   /* copy buildid from user space */
+   if (strncpy_from_user(id, u64_to_user_ptr(attr->kern_buildid),
+ buildid.size) < 0)
+   return -EFAULT;
+
+   return memcmp(id, buildid.ptr, buildid.size);
+}
+
+static int check_buildid(union bpf_attr *attr)
+{
+   return buildid.ptr ? __check_buildid(attr) : 0;
+}
+
+#define NT_ALIGN(n)(((n) + 3) & ~3)
+#define NT_GNU_BUILD_ID3
+
+extern const void __start_notes __weak;
+extern const void __stop_notes __weak;
+
+static int __init buildid_init(void)
+{
+   const void *ptr = &__start_notes;
+
+   while (ptr < &__stop_notes) {
+   const Elf64_Nhdr *nhdr = ptr;
+   size_t namesz = NT_ALIGN(nhdr->n_namesz),
+  descsz = NT_ALIGN(nhdr->n_descsz);
+   const char *name;
+
+   ptr += sizeof(*nhdr);
+   name = ptr;
+   ptr += namesz;
+
+   if (nhdr->n_type != NT_GNU_BUILD_ID ||
+   nhdr->n_namesz != sizeof("GNU") ||
+   memcmp(name, "GNU", sizeof("GNU"))) {
+   ptr += descsz;
+   continue;
+   }
+
+   buildid.ptr = ptr;
+   buildid.size = descsz;
+   break;
+   }
+
+   /* Sanity checks on the parsed buildid. */
+   if (!buildid.ptr) {
+   pr_warn("bpf: GNU buildid not found, switching off the 
check\n");
+   } else if (buildid.size > 64) {
+   pr_warn("bpf: GNU buildid too long, switching off the check\n");
+   buildid.ptr = NULL;
+   }
+
+   return 0;
+}
+
+subsys_initcall(buildid_init);
+
+#else
+static int check_buildid(union bpf_attr *attr __maybe_unused)
+{
+   return 0;
+}
+#endif /* CONFIG_BPF_BUILDID_CHECK */
 
 static int bpf_prog_load(union bpf_attr *attr)
 {
@@ -1271,6 +1349,10 @@ static int bpf_prog_load(union bpf_attr *attr)
attr->kern_version != LINUX_VERSION_CODE)
return -EINVAL;
 
+   if (type == BPF_PROG_TYPE_KPROBE &&
+   check_buildid(attr))
+   return -EINVAL;
+
if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
type != BPF_PROG_TYPE_CGROUP_SKB &&
!capable(CAP_SYS_ADMIN))
-- 
2.13.6

[PATCH 8/9] libbpf: Add support to attach buildid to program load

[Jiri Olsa]

Adding support to retrieve buildid from elf's "buildid"
section and passing it through to the load_program
function to kernel bpf syscall.

Fixing perf use of the bpf_load_program function and
linking in the vsprintf.o into bpftool to have the
scnprintf function in.

Link: http://lkml.kernel.org/n/tip-2pafwtzbyosmf9ftuf0ud...@git.kernel.org
Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/Makefile |  5 -
 tools/lib/bpf/bpf.c|  6 --
 tools/lib/bpf/bpf.h|  5 +++--
 tools/lib/bpf/libbpf.c | 46 --
 tools/perf/tests/bpf.c |  9 -
 5 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
index 4e69782c4a79..9ac11ea5de1c 100644
--- a/tools/bpf/bpftool/Makefile
+++ b/tools/bpf/bpftool/Makefile
@@ -75,11 +75,14 @@ include $(wildcard $(OUTPUT)*.d)
 all: $(OUTPUT)bpftool
 
 SRCS = $(wildcard *.c)
-OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o
+OBJS = $(patsubst %.c,$(OUTPUT)%.o,$(SRCS)) $(OUTPUT)disasm.o 
$(OUTPUT)vsprintf.o
 
 $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
$(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
 
+$(OUTPUT)vsprintf.o: $(srctree)/tools/lib/vsprintf.c
+   $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
+
 $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
$(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
 
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index acbb3f8b3bec..8e384db5bbbd 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -168,6 +168,7 @@ int bpf_load_program_xattr(const struct 
bpf_load_program_attr *load_attr,
attr.log_size = 0;
attr.log_level = 0;
attr.kern_version = load_attr->kern_version;
+   attr.kern_buildid = ptr_to_u64(load_attr->buildid);
memcpy(attr.prog_name, load_attr->name,
   min(name_len, BPF_OBJ_NAME_LEN - 1));
 
@@ -185,8 +186,8 @@ int bpf_load_program_xattr(const struct 
bpf_load_program_attr *load_attr,
 
 int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
 size_t insns_cnt, const char *license,
-__u32 kern_version, char *log_buf,
-size_t log_buf_sz)
+__u32 kern_version, const char *buildid,
+char *log_buf, size_t log_buf_sz)
 {
struct bpf_load_program_attr load_attr;
 
@@ -198,6 +199,7 @@ int bpf_load_program(enum bpf_prog_type type, const struct 
bpf_insn *insns,
load_attr.insns_cnt = insns_cnt;
load_attr.license = license;
load_attr.kern_version = kern_version;
+   load_attr.buildid = buildid;
 
return bpf_load_program_xattr(_attr, log_buf, log_buf_sz);
 }
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 39f6a0d64a3b..b5ffb178ebdd 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -49,6 +49,7 @@ struct bpf_load_program_attr {
size_t insns_cnt;
const char *license;
__u32 kern_version;
+   const char *buildid;
 };
 
 /* Recommend log buffer size */
@@ -57,8 +58,8 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr 
*load_attr,
   char *log_buf, size_t log_buf_sz);
 int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
 size_t insns_cnt, const char *license,
-__u32 kern_version, char *log_buf,
-size_t log_buf_sz);
+__u32 kern_version, const char *buildid,
+char *log_buf, size_t log_buf_sz);
 int bpf_verify_program(enum bpf_prog_type type, const struct bpf_insn *insns,
   size_t insns_cnt, int strict_alignment,
   const char *license, __u32 kern_version,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5922443063f0..421f2c2e0ebe 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -220,6 +220,7 @@ static LIST_HEAD(bpf_objects_list);
 
 struct bpf_object {
char license[64];
+   char buildid[64];
u32 kern_version;
 
struct bpf_program *programs;
@@ -599,6 +600,32 @@ bpf_object__init_kversion(struct bpf_object *obj,
return 0;
 }
 
+static void buildid_scnprint(char *buf, int n, char *buildid, int size)
+{
+   int i, ret = 0;
+
+   for (i = 0; i < size; i++) {
+   ret += scnprintf(buf + ret, n - ret, "%x",
+(unsigned char) buildid[i]);
+   }
+}
+
+static int
+bpf_object__init_buildid(struct bpf_object *obj,
+void *data, size_t size)
+{
+   char buf[64];
+
+   if (size > sizeof(obj->buildid))
+   return -LIBBPF_ERRNO__FORMAT;
+
+   memcpy(>buildid, data, size);
+
+   buildid_scnprint(buf, 64, obj->buildid, size);
+   pr_debug("kernel buildid of %s is: %s\n", obj->path, buf);
+  

[PATCH 9/9] perf tools: The buildid usage in example eBPF program

[Jiri Olsa]

The bpf-samples/bpf-stdout-example.c demonstrates how to put the
buildid data into eBPF program.

Link: http://lkml.kernel.org/n/tip-dq97ddil7h3qbvphbbo8p...@git.kernel.org
Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/perf/bpf-samples/bpf-stdout-example.c | 42 +
 1 file changed, 42 insertions(+)
 create mode 100644 tools/perf/bpf-samples/bpf-stdout-example.c

diff --git a/tools/perf/bpf-samples/bpf-stdout-example.c 
b/tools/perf/bpf-samples/bpf-stdout-example.c
new file mode 100644
index ..60783442bd5c
--- /dev/null
+++ b/tools/perf/bpf-samples/bpf-stdout-example.c
@@ -0,0 +1,42 @@
+#include 
+#include 
+
+#define SEC(NAME) __attribute__((section(NAME), used))
+
+char _license[] SEC("license") = "GPL";
+int _versionSEC("version") = LINUX_VERSION_CODE;
+char _buildid[] SEC("buildid") = LINUX_BUILDID_DATA;
+
+static unsigned long long (*bpf_get_smp_processor_id)(void) =
+   (void *) BPF_FUNC_get_smp_processor_id;
+static int (*bpf_perf_event_output)(void *ctx, void *map,
+unsigned long long flags, void *data,
+int size) =
+(void *) BPF_FUNC_perf_event_output;
+
+struct bpf_map_def {
+unsigned int type;
+unsigned int key_size;
+unsigned int value_size;
+unsigned int max_entries;
+unsigned int map_flags;
+unsigned int inner_map_idx;
+unsigned int numa_node;
+};
+
+struct bpf_map_def SEC("maps") __bpf_stdout__ = {
+   .type   = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+   .key_size   = sizeof(int),
+   .value_size = sizeof(u32),
+   .max_entries= __NR_CPUS__,
+};
+
+SEC("probe=sys_read")
+int func(void *ctx)
+{
+   char output_str[] = "Raise a BPF event!";
+
+   bpf_perf_event_output(ctx, &__bpf_stdout__, bpf_get_smp_processor_id(),
+ _str, sizeof(output_str));
+   return 0;
+}
-- 
2.13.6

[PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

We use print_bpf_insn in user space (bpftool and soon perf),
so it'd be nice to keep it generic and strip it off the kernel
struct bpf_verifier_env argument.

This argument can be safely removed, because its users can
use the struct bpf_insn_cbs::private_data to pass it.

By changing the argument type  we can no longer have clean
'verbose' alias to 'bpf_verifier_log_write' in verifier.c.
Instead  we're adding the  'verbose' cb_print callback and
removing the alias.

This way we have new cb_print callback in place, and all
the 'verbose(env, ...) calls in verifier.c will cleanly
cast to 'verbose(void *, ...)' so no other change is
needed.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 kernel/bpf/disasm.c   | 52 +--
 kernel/bpf/disasm.h   |  5 +
 kernel/bpf/verifier.c | 44 ++-
 3 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
index 8740406df2cd..d6b76377cb6e 100644
--- a/kernel/bpf/disasm.c
+++ b/kernel/bpf/disasm.c
@@ -113,16 +113,16 @@ static const char *const bpf_jmp_string[16] = {
 };
 
 static void print_bpf_end_insn(bpf_insn_print_t verbose,
-  struct bpf_verifier_env *env,
+  void *private_data,
   const struct bpf_insn *insn)
 {
-   verbose(env, "(%02x) r%d = %s%d r%d\n", insn->code, insn->dst_reg,
+   verbose(private_data, "(%02x) r%d = %s%d r%d\n",
+   insn->code, insn->dst_reg,
BPF_SRC(insn->code) == BPF_TO_BE ? "be" : "le",
insn->imm, insn->dst_reg);
 }
 
 void print_bpf_insn(const struct bpf_insn_cbs *cbs,
-   struct bpf_verifier_env *env,
const struct bpf_insn *insn,
bool allow_ptr_leaks)
 {
@@ -132,23 +132,23 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
if (class == BPF_ALU || class == BPF_ALU64) {
if (BPF_OP(insn->code) == BPF_END) {
if (class == BPF_ALU64)
-   verbose(env, "BUG_alu64_%02x\n", insn->code);
+   verbose(cbs->private_data, "BUG_alu64_%02x\n", 
insn->code);
else
-   print_bpf_end_insn(verbose, env, insn);
+   print_bpf_end_insn(verbose, cbs->private_data, 
insn);
} else if (BPF_OP(insn->code) == BPF_NEG) {
-   verbose(env, "(%02x) r%d = %s-r%d\n",
+   verbose(cbs->private_data, "(%02x) r%d = %s-r%d\n",
insn->code, insn->dst_reg,
class == BPF_ALU ? "(u32) " : "",
insn->dst_reg);
} else if (BPF_SRC(insn->code) == BPF_X) {
-   verbose(env, "(%02x) %sr%d %s %sr%d\n",
+   verbose(cbs->private_data, "(%02x) %sr%d %s %sr%d\n",
insn->code, class == BPF_ALU ? "(u32) " : "",
insn->dst_reg,
bpf_alu_string[BPF_OP(insn->code) >> 4],
class == BPF_ALU ? "(u32) " : "",
insn->src_reg);
} else {
-   verbose(env, "(%02x) %sr%d %s %s%d\n",
+   verbose(cbs->private_data, "(%02x) %sr%d %s %s%d\n",
insn->code, class == BPF_ALU ? "(u32) " : "",
insn->dst_reg,
bpf_alu_string[BPF_OP(insn->code) >> 4],
@@ -157,46 +157,46 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
}
} else if (class == BPF_STX) {
if (BPF_MODE(insn->code) == BPF_MEM)
-   verbose(env, "(%02x) *(%s *)(r%d %+d) = r%d\n",
+   verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = 
r%d\n",
insn->code,
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
insn->dst_reg,
insn->off, insn->src_reg);
else if (BPF_MODE(insn->code) == BPF_XADD)
-   verbose(env, "(%02x) lock *(%s *)(r%d %+d) += r%d\n",
+   verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d 
%+d) += r%d\n",
insn->code,
bpf_ldst_string[BPF_SIZE(in

[PATCH 0/2] bpf: Change print_bpf_insn interface

[Jiri Olsa]

hi,
this patchset removes struct bpf_verifier_env argument
from print_bpf_insn function (patch 1) and changes user
space bpftool user to use it that way (patch 2).

thanks,
jirka

---
Jiri Olsa (2):
  bpf: Remove struct bpf_verifier_env argument from print_bpf_insn
  bpftool: Adjust to new print_bpf_insn interface

 kernel/bpf/disasm.c   | 52 
++--
 kernel/bpf/disasm.h   |  5 +
 kernel/bpf/verifier.c | 44 
+++-
 tools/bpf/bpftool/xlated_dumper.c | 12 ++--
 4 files changed, 60 insertions(+), 53 deletions(-)

[PATCH 2/2] bpftool: Adjust to new print_bpf_insn interface

[Jiri Olsa]

Change bpftool to skip the removed struct bpf_verifier_env
argument in print_bpf_insn. It was passed as NULL anyway.

No functional change intended.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/xlated_dumper.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/xlated_dumper.c 
b/tools/bpf/bpftool/xlated_dumper.c
index 20da835e9e38..7a3173b76c16 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -114,7 +114,7 @@ static struct kernel_sym *kernel_syms_search(struct 
dump_data *dd,
   sizeof(*dd->sym_mapping), kernel_syms_cmp) : NULL;
 }
 
-static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn(void *private_data, const char *fmt, ...)
 {
va_list args;
 
@@ -124,7 +124,7 @@ static void print_insn(struct bpf_verifier_env *env, const 
char *fmt, ...)
 }
 
 static void
-print_insn_for_graph(struct bpf_verifier_env *env, const char *fmt, ...)
+print_insn_for_graph(void *private_data, const char *fmt, ...)
 {
char buf[64], *p;
va_list args;
@@ -154,7 +154,7 @@ print_insn_for_graph(struct bpf_verifier_env *env, const 
char *fmt, ...)
printf("%s", buf);
 }
 
-static void print_insn_json(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn_json(void *private_data, const char *fmt, ...)
 {
unsigned int l = strlen(fmt);
char chomped_fmt[l];
@@ -248,7 +248,7 @@ void dump_xlated_json(struct dump_data *dd, void *buf, 
unsigned int len,
 
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "disasm");
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
jsonw_name(json_wtr, "opcodes");
@@ -302,7 +302,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, 
unsigned int len,
double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
 
printf("% 4d: ", i);
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
printf("   ");
@@ -331,7 +331,7 @@ void dump_xlated_for_graph(struct dump_data *dd, void 
*buf_start, void *buf_end,
 
for (; cur <= insn_end; cur++) {
printf("% 4d: ", (int)(cur - insn_start + start_idx));
-   print_bpf_insn(, NULL, cur, true);
+   print_bpf_insn(, cur, true);
if (cur != insn_end)
printf(" | ");
}
-- 
2.13.6

Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

On Thu, Mar 22, 2018 at 05:07:48PM +0100, Daniel Borkmann wrote:

SNIP

> >>> + va_end(args);
> >>> +}
> >>>  EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
> >>> +
> >>> +__printf(2, 3) static void print_ins(void *private_data,
> >>> +  const char *fmt, ...)
> >>
> >> Unless I am mistaken, you could just call this one "verbose()" and
> >> simply remove the function alias?
> > 
> > right you are ;-) I haven't realized that struct bpf_verifier_env *env
> > will cleanly cast to void * 
> > 
> > new version attached.. I'll make some tests and send full patch
> 
> When you do so, please make sure to send a full fresh series with the two
> patches and also cover letter included, otherwise it's more fragile wrt
> potentially applying the wrong patch from one of the replies. :-)

will do, thanks

jirka

Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

On Thu, Mar 22, 2018 at 03:35:42PM +, Quentin Monnet wrote:
> 2018-03-22 14:32 UTC+0100 ~ Jiri Olsa <jo...@redhat.com>
> > On Thu, Mar 22, 2018 at 10:34:18AM +0100, Daniel Borkmann wrote:
> >> On 03/21/2018 07:37 PM, Jiri Olsa wrote:
> >>> On Wed, Mar 21, 2018 at 05:25:33PM +, Quentin Monnet wrote:
> >>>> 2018-03-21 16:02 UTC+0100 ~ Jiri Olsa <jo...@kernel.org>
> >>>>> We use print_bpf_insn in user space (bpftool and soon perf),
> >>>>> so it'd be nice to keep it generic and strip it off the kernel
> >>>>> struct bpf_verifier_env argument.
> >>>>>
> >>>>> This argument can be safely removed, because its users can
> >>>>> use the struct bpf_insn_cbs::private_data to pass it.
> >>>>>
> >>>>> Signed-off-by: Jiri Olsa <jo...@kernel.org>
> >>>>> ---
> >>>>>  kernel/bpf/disasm.c   | 52 
> >>>>> +--
> >>>>>  kernel/bpf/disasm.h   |  5 +
> >>>>>  kernel/bpf/verifier.c |  6 +++---
> >>>>>  3 files changed, 30 insertions(+), 33 deletions(-)
> >>>>
> >>>> [...]
> >>>>
> >>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >>>>> index c6eff108aa99..9f27d3fa7259 100644
> >>>>> --- a/kernel/bpf/verifier.c
> >>>>> +++ b/kernel/bpf/verifier.c
> >>>>> @@ -202,8 +202,7 @@ EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
> >>>>>   * generic for symbol export. The function was renamed, but not the 
> >>>>> calls in
> >>>>>   * the verifier to avoid complicating backports. Hence the alias below.
> >>>>>   */
> >>>>> -static __printf(2, 3) void verbose(struct bpf_verifier_env *env,
> >>>>> -  const char *fmt, ...)
> >>>>> +static __printf(2, 3) void verbose(void *private_data, const char 
> >>>>> *fmt, ...)
> >>>>> __attribute__((alias("bpf_verifier_log_write")));
> >>>>
> >>>> Just as a note, verbose() will be aliased to a function whose prototype
> >>>> differs (bpf_verifier_log_write() still expects a struct
> >>>> bpf_verifier_env as its first argument). I am not so familiar with
> >>>> function aliases, could this change be a concern?
> >>>
> >>> yea, but as it was pointer for pointer switch I did not
> >>> see any problem with that.. I'll check more
> >>
> >> Ok, holding off for now until we have clarification. Other option could 
> >> also
> >> be to make it void *private_data everywhere and for the kernel writer then
> >> do struct bpf_verifier_env *env = private_data.
> > 
> > can't find much info about the alias behaviour for this
> > case.. so how about having separate function for the
> > print_cb like below.. I still need to test it
> 
> I have nothing against splitting the function. This is a solution I
> considered for verbose() and bpf_verifier_log_write(), before switching
> to function alias (on Daniel's suggestion) because the code would be
> identical for the two separate functions at that time. But with your
> change they would not have the same signature anymore, so it sounds
> reasonable. Just a note below...
> 
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> >  kernel/bpf/disasm.c   | 52 
> > +--
> >  kernel/bpf/disasm.h   |  5 +
> >  kernel/bpf/verifier.c | 41 +---
> >  3 files changed, 57 insertions(+), 41 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index e9f7c20691c1..69bf7590877c 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -168,23 +168,16 @@ struct bpf_call_arg_meta {
> >  
> >  static DEFINE_MUTEX(bpf_verifier_lock);
> >  
> > -/* log_level controls verbosity level of eBPF verifier.
> > - * bpf_verifier_log_write() is used to dump the verification trace to the 
> > log,
> > - * so the user can figure out what's wrong with the program
> > - */
> > -__printf(2, 3) void bpf_verifier_log_write(struct bpf_verifier_env *env,
> > -  const char *fmt, ...)
> > +static void log_write(struct

Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

On Thu, Mar 22, 2018 at 10:34:18AM +0100, Daniel Borkmann wrote:
> On 03/21/2018 07:37 PM, Jiri Olsa wrote:
> > On Wed, Mar 21, 2018 at 05:25:33PM +, Quentin Monnet wrote:
> >> 2018-03-21 16:02 UTC+0100 ~ Jiri Olsa <jo...@kernel.org>
> >>> We use print_bpf_insn in user space (bpftool and soon perf),
> >>> so it'd be nice to keep it generic and strip it off the kernel
> >>> struct bpf_verifier_env argument.
> >>>
> >>> This argument can be safely removed, because its users can
> >>> use the struct bpf_insn_cbs::private_data to pass it.
> >>>
> >>> Signed-off-by: Jiri Olsa <jo...@kernel.org>
> >>> ---
> >>>  kernel/bpf/disasm.c   | 52 
> >>> +--
> >>>  kernel/bpf/disasm.h   |  5 +
> >>>  kernel/bpf/verifier.c |  6 +++---
> >>>  3 files changed, 30 insertions(+), 33 deletions(-)
> >>
> >> [...]
> >>
> >>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >>> index c6eff108aa99..9f27d3fa7259 100644
> >>> --- a/kernel/bpf/verifier.c
> >>> +++ b/kernel/bpf/verifier.c
> >>> @@ -202,8 +202,7 @@ EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
> >>>   * generic for symbol export. The function was renamed, but not the 
> >>> calls in
> >>>   * the verifier to avoid complicating backports. Hence the alias below.
> >>>   */
> >>> -static __printf(2, 3) void verbose(struct bpf_verifier_env *env,
> >>> -const char *fmt, ...)
> >>> +static __printf(2, 3) void verbose(void *private_data, const char *fmt, 
> >>> ...)
> >>>   __attribute__((alias("bpf_verifier_log_write")));
> >>
> >> Just as a note, verbose() will be aliased to a function whose prototype
> >> differs (bpf_verifier_log_write() still expects a struct
> >> bpf_verifier_env as its first argument). I am not so familiar with
> >> function aliases, could this change be a concern?
> > 
> > yea, but as it was pointer for pointer switch I did not
> > see any problem with that.. I'll check more
> 
> Ok, holding off for now until we have clarification. Other option could also
> be to make it void *private_data everywhere and for the kernel writer then
> do struct bpf_verifier_env *env = private_data.

can't find much info about the alias behaviour for this
case.. so how about having separate function for the
print_cb like below.. I still need to test it

thanks,
jirka


---
 kernel/bpf/disasm.c   | 52 +--
 kernel/bpf/disasm.h   |  5 +
 kernel/bpf/verifier.c | 41 +---
 3 files changed, 57 insertions(+), 41 deletions(-)

diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
index 8740406df2cd..d6b76377cb6e 100644
--- a/kernel/bpf/disasm.c
+++ b/kernel/bpf/disasm.c
@@ -113,16 +113,16 @@ static const char *const bpf_jmp_string[16] = {
 };
 
 static void print_bpf_end_insn(bpf_insn_print_t verbose,
-  struct bpf_verifier_env *env,
+  void *private_data,
   const struct bpf_insn *insn)
 {
-   verbose(env, "(%02x) r%d = %s%d r%d\n", insn->code, insn->dst_reg,
+   verbose(private_data, "(%02x) r%d = %s%d r%d\n",
+   insn->code, insn->dst_reg,
BPF_SRC(insn->code) == BPF_TO_BE ? "be" : "le",
insn->imm, insn->dst_reg);
 }
 
 void print_bpf_insn(const struct bpf_insn_cbs *cbs,
-   struct bpf_verifier_env *env,
const struct bpf_insn *insn,
bool allow_ptr_leaks)
 {
@@ -132,23 +132,23 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
if (class == BPF_ALU || class == BPF_ALU64) {
if (BPF_OP(insn->code) == BPF_END) {
if (class == BPF_ALU64)
-   verbose(env, "BUG_alu64_%02x\n", insn->code);
+   verbose(cbs->private_data, "BUG_alu64_%02x\n", 
insn->code);
else
-   print_bpf_end_insn(verbose, env, insn);
+   print_bpf_end_insn(verbose, cbs->private_data, 
insn);
} else if (BPF_OP(insn->code) == BPF_NEG) {
-   verbose(env, "(%02x) r%d = %s-r%d\n",
+   verbose(cbs->private_data, "(%02x) r%d = %s-r%d\n",
insn->code, insn->dst_reg,
  

Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

On Wed, Mar 21, 2018 at 05:25:33PM +, Quentin Monnet wrote:
> 2018-03-21 16:02 UTC+0100 ~ Jiri Olsa <jo...@kernel.org>
> > We use print_bpf_insn in user space (bpftool and soon perf),
> > so it'd be nice to keep it generic and strip it off the kernel
> > struct bpf_verifier_env argument.
> > 
> > This argument can be safely removed, because its users can
> > use the struct bpf_insn_cbs::private_data to pass it.
> > 
> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> > ---
> >  kernel/bpf/disasm.c   | 52 
> > +--
> >  kernel/bpf/disasm.h   |  5 +
> >  kernel/bpf/verifier.c |  6 +++---
> >  3 files changed, 30 insertions(+), 33 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index c6eff108aa99..9f27d3fa7259 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -202,8 +202,7 @@ EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
> >   * generic for symbol export. The function was renamed, but not the calls 
> > in
> >   * the verifier to avoid complicating backports. Hence the alias below.
> >   */
> > -static __printf(2, 3) void verbose(struct bpf_verifier_env *env,
> > -  const char *fmt, ...)
> > +static __printf(2, 3) void verbose(void *private_data, const char *fmt, 
> > ...)
> > __attribute__((alias("bpf_verifier_log_write")));
> 
> Just as a note, verbose() will be aliased to a function whose prototype
> differs (bpf_verifier_log_write() still expects a struct
> bpf_verifier_env as its first argument). I am not so familiar with
> function aliases, could this change be a concern?

yea, but as it was pointer for pointer switch I did not
see any problem with that.. I'll check more

jirka

[PATCHv2 2/2] bpftool: Adjust to new print_bpf_insn interface

[Jiri Olsa]

On Wed, Mar 21, 2018 at 05:44:52PM +0100, Daniel Borkmann wrote:

SNIP

> >> Hi Jiri, this code has changed in the tree. It was moved to
> >> tools/bpf/bpftool/xlated_dumper.c, and there is now a third function to
> >> update: print_insn_for_graph(). Could you please rebase the patch?
> > 
> > sure.. I was over perf tree, I'll check on bpf tree
> 
> Just to be sure, it should be bpf-next. bpf is for fixes only.

v2 attached

thanks,
jirka


---
Change bpftool to skip the removed struct bpf_verifier_env
argument in print_bpf_insn. It was passed as NULL anyway.

No functional change intended.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/xlated_dumper.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/bpf/bpftool/xlated_dumper.c 
b/tools/bpf/bpftool/xlated_dumper.c
index 20da835e9e38..7a3173b76c16 100644
--- a/tools/bpf/bpftool/xlated_dumper.c
+++ b/tools/bpf/bpftool/xlated_dumper.c
@@ -114,7 +114,7 @@ static struct kernel_sym *kernel_syms_search(struct 
dump_data *dd,
   sizeof(*dd->sym_mapping), kernel_syms_cmp) : NULL;
 }
 
-static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn(void *private_data, const char *fmt, ...)
 {
va_list args;
 
@@ -124,7 +124,7 @@ static void print_insn(struct bpf_verifier_env *env, const 
char *fmt, ...)
 }
 
 static void
-print_insn_for_graph(struct bpf_verifier_env *env, const char *fmt, ...)
+print_insn_for_graph(void *private_data, const char *fmt, ...)
 {
char buf[64], *p;
va_list args;
@@ -154,7 +154,7 @@ print_insn_for_graph(struct bpf_verifier_env *env, const 
char *fmt, ...)
printf("%s", buf);
 }
 
-static void print_insn_json(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn_json(void *private_data, const char *fmt, ...)
 {
unsigned int l = strlen(fmt);
char chomped_fmt[l];
@@ -248,7 +248,7 @@ void dump_xlated_json(struct dump_data *dd, void *buf, 
unsigned int len,
 
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "disasm");
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
jsonw_name(json_wtr, "opcodes");
@@ -302,7 +302,7 @@ void dump_xlated_plain(struct dump_data *dd, void *buf, 
unsigned int len,
double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
 
printf("% 4d: ", i);
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
printf("   ");
@@ -331,7 +331,7 @@ void dump_xlated_for_graph(struct dump_data *dd, void 
*buf_start, void *buf_end,
 
for (; cur <= insn_end; cur++) {
printf("% 4d: ", (int)(cur - insn_start + start_idx));
-   print_bpf_insn(, NULL, cur, true);
+   print_bpf_insn(, cur, true);
if (cur != insn_end)
printf(" | ");
}
-- 
2.13.6

Re: [PATCH 2/2] bpftool: Adjust to new print_bpf_insn interface

[Jiri Olsa]

On Wed, Mar 21, 2018 at 04:39:09PM +, Quentin Monnet wrote:
> 2018-03-21 16:02 UTC+0100 ~ Jiri Olsa <jo...@kernel.org>
> > Change bpftool to skip the removed struct bpf_verifier_env
> > argument in print_bpf_insn. It was passed as NULL anyway.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Jiri Olsa <jo...@kernel.org>
> > ---
> >  tools/bpf/bpftool/prog.c | 8 
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> > index e549e329be82..108001d974ee 100644
> > --- a/tools/bpf/bpftool/prog.c
> > +++ b/tools/bpf/bpftool/prog.c
> > @@ -489,7 +489,7 @@ static struct kernel_sym *kernel_syms_search(struct 
> > dump_data *dd,
> >sizeof(*dd->sym_mapping), kernel_syms_cmp) : NULL;
> >  }
> >  
> > -static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
> > +static void print_insn(void *private_data, const char *fmt, ...)
> >  {
> > va_list args;
> >  
> > @@ -576,7 +576,7 @@ static void dump_xlated_plain(struct dump_data *dd, 
> > void *buf,
> > double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
> >  
> > printf("% 4d: ", i);
> > -   print_bpf_insn(, NULL, insn + i, true);
> > +   print_bpf_insn(, insn + i, true);
> >  
> > if (opcodes) {
> > printf("   ");
> > @@ -590,7 +590,7 @@ static void dump_xlated_plain(struct dump_data *dd, 
> > void *buf,
> > }
> >  }
> >  
> > -static void print_insn_json(struct bpf_verifier_env *env, const char *fmt, 
> > ...)
> > +static void print_insn_json(void *private_data, const char *fmt, ...)
> >  {
> > unsigned int l = strlen(fmt);
> > char chomped_fmt[l];
> > @@ -628,7 +628,7 @@ static void dump_xlated_json(struct dump_data *dd, void 
> > *buf,
> >  
> > jsonw_start_object(json_wtr);
> > jsonw_name(json_wtr, "disasm");
> > -   print_bpf_insn(, NULL, insn + i, true);
> > +   print_bpf_insn(, insn + i, true);
> >  
> > if (opcodes) {
> > jsonw_name(json_wtr, "opcodes");
> > 
> 
> Hi Jiri, this code has changed in the tree. It was moved to
> tools/bpf/bpftool/xlated_dumper.c, and there is now a third function to
> update: print_insn_for_graph(). Could you please rebase the patch?

sure.. I was over perf tree, I'll check on bpf tree

thanks,
jirka

[PATCH 2/2] bpftool: Adjust to new print_bpf_insn interface

[Jiri Olsa]

Change bpftool to skip the removed struct bpf_verifier_env
argument in print_bpf_insn. It was passed as NULL anyway.

No functional change intended.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 tools/bpf/bpftool/prog.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index e549e329be82..108001d974ee 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -489,7 +489,7 @@ static struct kernel_sym *kernel_syms_search(struct 
dump_data *dd,
   sizeof(*dd->sym_mapping), kernel_syms_cmp) : NULL;
 }
 
-static void print_insn(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn(void *private_data, const char *fmt, ...)
 {
va_list args;
 
@@ -576,7 +576,7 @@ static void dump_xlated_plain(struct dump_data *dd, void 
*buf,
double_insn = insn[i].code == (BPF_LD | BPF_IMM | BPF_DW);
 
printf("% 4d: ", i);
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
printf("   ");
@@ -590,7 +590,7 @@ static void dump_xlated_plain(struct dump_data *dd, void 
*buf,
}
 }
 
-static void print_insn_json(struct bpf_verifier_env *env, const char *fmt, ...)
+static void print_insn_json(void *private_data, const char *fmt, ...)
 {
unsigned int l = strlen(fmt);
char chomped_fmt[l];
@@ -628,7 +628,7 @@ static void dump_xlated_json(struct dump_data *dd, void 
*buf,
 
jsonw_start_object(json_wtr);
jsonw_name(json_wtr, "disasm");
-   print_bpf_insn(, NULL, insn + i, true);
+   print_bpf_insn(, insn + i, true);
 
if (opcodes) {
jsonw_name(json_wtr, "opcodes");
-- 
2.13.6

[PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf_insn

[Jiri Olsa]

We use print_bpf_insn in user space (bpftool and soon perf),
so it'd be nice to keep it generic and strip it off the kernel
struct bpf_verifier_env argument.

This argument can be safely removed, because its users can
use the struct bpf_insn_cbs::private_data to pass it.

Signed-off-by: Jiri Olsa <jo...@kernel.org>
---
 kernel/bpf/disasm.c   | 52 +--
 kernel/bpf/disasm.h   |  5 +
 kernel/bpf/verifier.c |  6 +++---
 3 files changed, 30 insertions(+), 33 deletions(-)

diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
index 8740406df2cd..d6b76377cb6e 100644
--- a/kernel/bpf/disasm.c
+++ b/kernel/bpf/disasm.c
@@ -113,16 +113,16 @@ static const char *const bpf_jmp_string[16] = {
 };
 
 static void print_bpf_end_insn(bpf_insn_print_t verbose,
-  struct bpf_verifier_env *env,
+  void *private_data,
   const struct bpf_insn *insn)
 {
-   verbose(env, "(%02x) r%d = %s%d r%d\n", insn->code, insn->dst_reg,
+   verbose(private_data, "(%02x) r%d = %s%d r%d\n",
+   insn->code, insn->dst_reg,
BPF_SRC(insn->code) == BPF_TO_BE ? "be" : "le",
insn->imm, insn->dst_reg);
 }
 
 void print_bpf_insn(const struct bpf_insn_cbs *cbs,
-   struct bpf_verifier_env *env,
const struct bpf_insn *insn,
bool allow_ptr_leaks)
 {
@@ -132,23 +132,23 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
if (class == BPF_ALU || class == BPF_ALU64) {
if (BPF_OP(insn->code) == BPF_END) {
if (class == BPF_ALU64)
-   verbose(env, "BUG_alu64_%02x\n", insn->code);
+   verbose(cbs->private_data, "BUG_alu64_%02x\n", 
insn->code);
else
-   print_bpf_end_insn(verbose, env, insn);
+   print_bpf_end_insn(verbose, cbs->private_data, 
insn);
} else if (BPF_OP(insn->code) == BPF_NEG) {
-   verbose(env, "(%02x) r%d = %s-r%d\n",
+   verbose(cbs->private_data, "(%02x) r%d = %s-r%d\n",
insn->code, insn->dst_reg,
class == BPF_ALU ? "(u32) " : "",
insn->dst_reg);
} else if (BPF_SRC(insn->code) == BPF_X) {
-   verbose(env, "(%02x) %sr%d %s %sr%d\n",
+   verbose(cbs->private_data, "(%02x) %sr%d %s %sr%d\n",
insn->code, class == BPF_ALU ? "(u32) " : "",
insn->dst_reg,
bpf_alu_string[BPF_OP(insn->code) >> 4],
class == BPF_ALU ? "(u32) " : "",
insn->src_reg);
} else {
-   verbose(env, "(%02x) %sr%d %s %s%d\n",
+   verbose(cbs->private_data, "(%02x) %sr%d %s %s%d\n",
insn->code, class == BPF_ALU ? "(u32) " : "",
insn->dst_reg,
bpf_alu_string[BPF_OP(insn->code) >> 4],
@@ -157,46 +157,46 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
}
} else if (class == BPF_STX) {
if (BPF_MODE(insn->code) == BPF_MEM)
-   verbose(env, "(%02x) *(%s *)(r%d %+d) = r%d\n",
+   verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = 
r%d\n",
insn->code,
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
insn->dst_reg,
insn->off, insn->src_reg);
else if (BPF_MODE(insn->code) == BPF_XADD)
-   verbose(env, "(%02x) lock *(%s *)(r%d %+d) += r%d\n",
+   verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d 
%+d) += r%d\n",
insn->code,
bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
insn->dst_reg, insn->off,
insn->src_reg);
else
-   verbose(env, "BUG_%02x\n", insn->code);
+   verbose(cbs->private_data, "BUG_%02x\n", insn->code);
} else if (class == BPF_ST) {
if (BPF_MODE(insn->code) !=

Re: linux-next: build failure after merge of the net tree

[Jiri Olsa]

On Tue, Feb 14, 2017 at 09:50:20AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> 
> What I think Ingo meant with dependency at the build system level is to
> somehow state that if file A gets changed, then tool B must be rebuilt.
> 
> Now that samples/bpf and tools/perf/ depend on tools/lib/bpf/ I _always_
> build both, ditto for tools/objtool, that shares a different library
> with tools/perf/, tools/lib/subcmd/:
> 
> ENTRYPOINT make -C /git/linux/tools/perf O=/tmp/build/perf && \
>rm -rf /tmp/build/perf/{.[^.]*,*} && \
>make NO_LIBELF=1 -C /git/linux/tools/perf O=/tmp/build/perf && \
>make -C /git/linux/tools/objtool O=/tmp/build/objtool && \
>make -C /git/linux O=/tmp/build/linux allmodconfig && \
>make -C /git/linux O=/tmp/build/linux headers_install && \
>make -C /git/linux O=/tmp/build/linux samples/bpf/
> 
> This is the default action for my
> docker.io/acmel/linux-perf-tools-build-fedora:rawhide container.
> 
> It is published, so a:
> 
>docker pull docker.io/acmel/linux-perf-tools-build-fedora:rawhide
> 
> And then run it before pushing things upstream would catch these kinds
> of errors.
> 
> But that would possibly disrupt too much people's workflow, that is why
> using the Kbuild originated tools/build/ we have to somehow express that
> when a change is made in a file then a tool that uses that file needs to
> be rebuilt.

we already have the check in the check-headers.sh script,
an AFAICS there's no 'rebuild' option here.. just warn or fail
because the headers update needs to be done manualy

> 
> Makefile rules probably would be enough, but then it would have to be
> done at the tools/build/ level and all tools using shared components
> would have to use it to trigger the rebuild.

we can move/invoke the check-headers.sh script in some upper dir

jirka

Re: linux-next: build failure after merge of the net tree

[Jiri Olsa]

On Tue, Feb 14, 2017 at 07:42:21AM +0100, Ingo Molnar wrote:
> 
> * Stephen Rothwell  wrote:
> 
> > Hi all,
> > 
> > After merging the net tree, today's linux-next build (powerpc64le perf)
> > failed like this:
> > 
> > Warning: tools/include/uapi/linux/bpf.h differs from kernel
> > bpf.c: In function 'bpf_prog_attach':
> > bpf.c:180:6: error: 'union bpf_attr' has no member named 'attach_flags'; 
> > did you mean 'map_flags'?
> >   attr.attach_flags  = flags;
> >   ^
> > 
> > Caused by commit
> > 
> >   7f677633379b ("bpf: introduce BPF_F_ALLOW_OVERRIDE flag")
> > 
> > Unfortunately, the perf header files are kept separate from the kernel
> > header files proper and are not automatically copied over :-(
> 
> No, that's wrong, the problem is not that headers were not shared, the 
> problem is 
> that a tooling interdependency was not properly tested *and* that the 
> dependency 
> was not properly implemented in the build system either.
> 
> Note that we had similar build breakages when include headers _were_ shared 
> as 
> well, so sharing the headers would only have worked around this particular 
> bug and 
> would have introduced fragility in other places...
> 
> The best, most robust solution in this particular case would be to fix the 
> (tooling) build system to express the dependency, that would have shown the 
> build 
> failure right when the modification was done.

so we have the warning now:
  Warning: tools/include/uapi/linux/bpf.h differs from kernel

do you want to change it into the build failure?

jirka

Re: [RFC PATCH net-next 1/4] perf tools: Enable pre-event inherit setting by config terms

[Jiri Olsa]

On Wed, Oct 28, 2015 at 10:55:02AM +, Wang Nan wrote:

SNIP

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index f820906..397fb4e 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -653,6 +653,15 @@ static void apply_config_terms(struct perf_evsel *evsel,
>   case PERF_EVSEL__CONFIG_TERM_STACK_USER:
>   dump_size = term->val.stack_user;
>   break;
> + case PERF_EVSEL__CONFIG_TERM_INHERIT:
> + /*
> +  * attr->inherit should has already been set by
> +  * perf_evsel__config. If user explicitly set
> +  * inherit using config terms, override global
> +  * opt->no_inherit setting.
> +  */
> + attr->inherit = term->val.inherit ? 1 : 0;
> + break;
>   default:
>   break;
>   }
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 9a95e73..e402f83 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -43,6 +43,7 @@ enum {
>   PERF_EVSEL__CONFIG_TERM_TIME,
>   PERF_EVSEL__CONFIG_TERM_CALLGRAPH,
>   PERF_EVSEL__CONFIG_TERM_STACK_USER,
> + PERF_EVSEL__CONFIG_TERM_INHERIT,
>   PERF_EVSEL__CONFIG_TERM_MAX,
>  };
>  
> @@ -55,6 +56,7 @@ struct perf_evsel_config_term {
>   booltime;
>   char*callgraph;
>   u64 stack_user;
> + u64 inherit;

seems like bool would be enough

jirka
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH net-next 1/4] perf tools: Enable pre-event inherit setting by config terms

[Jiri Olsa]

On Wed, Oct 28, 2015 at 10:42:13AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 28, 2015 at 02:21:26PM +0100, Jiri Olsa escreveu:
> > On Wed, Oct 28, 2015 at 10:55:02AM +, Wang Nan wrote:
> > > @@ -55,6 +56,7 @@ struct perf_evsel_config_term {
> > >   booltime;
> > >   char*callgraph;
> > >   u64 stack_user;
> > > + u64 inherit;
> > 
> > seems like bool would be enough
> 
> Ok, will change this and move it to a more suitable place member
> alignment wise.
> 
> Can I, with this change, slap an Acked-by: jirka?

yep

jirka
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: Removing duplicit #includes

[Jiri Olsa]

Removing duplicit #includes for net/
Signed-off-by: Jiri Olsa [EMAIL PROTECTED] 
---

net/core/dst.c   |1 -
net/ieee80211/ieee80211_crypt_tkip.c |1 -
net/ieee80211/ieee80211_crypt_wep.c  |1 -
3 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/net/core/dst.c b/net/core/dst.c
index 16958e6..03daead 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -18,7 +18,6 @@
#include linux/types.h
#include net/net_namespace.h

-#include net/net_namespace.h
#include net/dst.h

/*
diff --git a/net/ieee80211/ieee80211_crypt_tkip.c 
b/net/ieee80211/ieee80211_crypt_tkip.c
index 4cce353..58b2261 100644
--- a/net/ieee80211/ieee80211_crypt_tkip.c
+++ b/net/ieee80211/ieee80211_crypt_tkip.c
@@ -25,7 +25,6 @@
#include net/ieee80211.h

#include linux/crypto.h
-#include linux/scatterlist.h
#include linux/crc32.h

MODULE_AUTHOR(Jouni Malinen);
diff --git a/net/ieee80211/ieee80211_crypt_wep.c 
b/net/ieee80211/ieee80211_crypt_wep.c
index 866fc04..3fa30c4 100644
--- a/net/ieee80211/ieee80211_crypt_wep.c
+++ b/net/ieee80211/ieee80211_crypt_wep.c
@@ -22,7 +22,6 @@
#include net/ieee80211.h

#include linux/crypto.h
-#include linux/scatterlist.h
#include linux/crc32.h

MODULE_AUTHOR(Jouni Malinen);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net: Removing duplicit #includes

[Jiri Olsa]

Removing duplicit #includes for net/
Signed-off-by: Jiri Olsa [EMAIL PROTECTED] 
---
 net/core/dst.c   |1 -
 net/ieee80211/ieee80211_crypt_tkip.c |1 -
 net/ieee80211/ieee80211_crypt_wep.c  |1 -
 3 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/net/core/dst.c b/net/core/dst.c
index 16958e6..03daead 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -18,7 +18,6 @@
 #include linux/types.h
 #include net/net_namespace.h
 
-#include net/net_namespace.h
 #include net/dst.h
 
 /*
diff --git a/net/ieee80211/ieee80211_crypt_tkip.c 
b/net/ieee80211/ieee80211_crypt_tkip.c
index 4cce353..58b2261 100644
--- a/net/ieee80211/ieee80211_crypt_tkip.c
+++ b/net/ieee80211/ieee80211_crypt_tkip.c
@@ -25,7 +25,6 @@
 #include net/ieee80211.h
 
 #include linux/crypto.h
-#include linux/scatterlist.h
 #include linux/crc32.h
 
 MODULE_AUTHOR(Jouni Malinen);
diff --git a/net/ieee80211/ieee80211_crypt_wep.c 
b/net/ieee80211/ieee80211_crypt_wep.c
index 866fc04..3fa30c4 100644
--- a/net/ieee80211/ieee80211_crypt_wep.c
+++ b/net/ieee80211/ieee80211_crypt_wep.c
@@ -22,7 +22,6 @@
 #include net/ieee80211.h
 
 #include linux/crypto.h
-#include linux/scatterlist.h
 #include linux/crc32.h
 
 MODULE_AUTHOR(Jouni Malinen);
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] net: Removing duplicit #includes

[Jiri Olsa]

sorry, the patch is mangled, I will resend another 

Jiri Olsa wrote:
 Removing duplicit #includes for net/
 Signed-off-by: Jiri Olsa [EMAIL PROTECTED] ---
 net/core/dst.c   |1 -
 net/ieee80211/ieee80211_crypt_tkip.c |1 -
 net/ieee80211/ieee80211_crypt_wep.c  |1 -
 3 files changed, 0 insertions(+), 3 deletions(-)
 
 diff --git a/net/core/dst.c b/net/core/dst.c
 index 16958e6..03daead 100644
 --- a/net/core/dst.c
 +++ b/net/core/dst.c
 @@ -18,7 +18,6 @@
 #include linux/types.h
 #include net/net_namespace.h
 
 -#include net/net_namespace.h
 #include net/dst.h
 
 /*
 diff --git a/net/ieee80211/ieee80211_crypt_tkip.c
 b/net/ieee80211/ieee80211_crypt_tkip.c
 index 4cce353..58b2261 100644
 --- a/net/ieee80211/ieee80211_crypt_tkip.c
 +++ b/net/ieee80211/ieee80211_crypt_tkip.c
 @@ -25,7 +25,6 @@
 #include net/ieee80211.h
 
 #include linux/crypto.h
 -#include linux/scatterlist.h
 #include linux/crc32.h
 
 MODULE_AUTHOR(Jouni Malinen);
 diff --git a/net/ieee80211/ieee80211_crypt_wep.c
 b/net/ieee80211/ieee80211_crypt_wep.c
 index 866fc04..3fa30c4 100644
 --- a/net/ieee80211/ieee80211_crypt_wep.c
 +++ b/net/ieee80211/ieee80211_crypt_wep.c
 @@ -22,7 +22,6 @@
 #include net/ieee80211.h
 
 #include linux/crypto.h
 -#include linux/scatterlist.h
 #include linux/crc32.h
 
 MODULE_AUTHOR(Jouni Malinen);
 

-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html