All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
@ 2018-10-04 13:50 Daniel Borkmann
  2018-10-04 17:11 ` Alexei Starovoitov
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Borkmann @ 2018-10-04 13:50 UTC (permalink / raw)
  To: ast; +Cc: netdev, brouer, Daniel Borkmann, Jiri Olsa

Allow for audit messages to be emitted upon BPF program load and
unload for having a timeline of events. The load itself is in
syscall context, so additional info about the process initiating
the BPF prog creation can be logged and later directly correlated
to the unload event.

The only info really needed from BPF side is the globally unique
prog ID where then audit user space tooling can query / dump all
info needed about the specific BPF program right upon load event
and enrich the record, thus these changes needed here can be kept
small and non-intrusive to the core.

Raw example output:

  # auditctl -D
  # auditctl -a always,exit -F arch=x86_64 -S bpf
  # ausearch --start recent -m 1332
  [...]
  ----
  time->Thu Oct  4 15:29:11 2018
  type=PROCTITLE msg=audit(1538659751.047:174): proctitle=2E2F746573745F76657269666965720030
  type=SYSCALL msg=audit(1538659751.047:174): arch=c000003e syscall=321 success=yes exit=3
    a0=5 a1=7ffe6f46dec0 a2=48 a3=40bd40 items=0 ppid=1892 pid=2020 auid=1000 uid=0 gid=0
    euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=3 comm="test_verifier"
    exe="/home/foo/test_verifier" key=(null)
  type=UNKNOWN[1332] msg=audit(1538659751.047:174): auid=1000 uid=0 gid=0 ses=3 pid=2020
    comm="test_verifier" exe="/home/foo/test_verifier" prog-id=2 event=LOAD
  ----
  time->Thu Oct  4 15:29:11 2018
  type=UNKNOWN[1332] msg=audit(1538659751.047:176): prog-id=2 event=UNLOAD
  ----
  [...]

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Olsa <jolsa@kernel.org>
---
 include/linux/audit.h      |  3 +++
 include/uapi/linux/audit.h |  1 +
 kernel/auditsc.c           |  2 +-
 kernel/bpf/syscall.c       | 31 +++++++++++++++++++++++++++++++
 4 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 9334fbe..9495c84 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -152,6 +152,7 @@ extern void		    audit_log_key(struct audit_buffer *ab,
 extern void		    audit_log_link_denied(const char *operation);
 extern void		    audit_log_lost(const char *message);
 
+extern void audit_log_task(struct audit_buffer *ab);
 extern int audit_log_task_context(struct audit_buffer *ab);
 extern void audit_log_task_info(struct audit_buffer *ab,
 				struct task_struct *tsk);
@@ -198,6 +199,8 @@ static inline void audit_log_key(struct audit_buffer *ab, char *key)
 { }
 static inline void audit_log_link_denied(const char *string)
 { }
+static inline void audit_log_task(struct audit_buffer *ab)
+{ }
 static inline int audit_log_task_context(struct audit_buffer *ab)
 {
 	return 0;
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 818ae69..a749881 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -114,6 +114,7 @@
 #define AUDIT_REPLACE		1329	/* Replace auditd if this packet unanswerd */
 #define AUDIT_KERN_MODULE	1330	/* Kernel Module events */
 #define AUDIT_FANOTIFY		1331	/* Fanotify access decision */
+#define AUDIT_BPF		1332	/* BPF subsystem */
 
 #define AUDIT_AVC		1400	/* SE Linux avc denial or grant */
 #define AUDIT_SELINUX_ERR	1401	/* Internal SE Linux Errors */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index b2d1f04..6c498d9 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2427,7 +2427,7 @@ void __audit_fanotify(unsigned int response)
 		AUDIT_FANOTIFY,	"resp=%u", response);
 }
 
-static void audit_log_task(struct audit_buffer *ab)
+void audit_log_task(struct audit_buffer *ab)
 {
 	kuid_t auid, uid;
 	kgid_t gid;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5742df2..83afbd1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -31,6 +31,7 @@
 #include <linux/timekeeping.h>
 #include <linux/ctype.h>
 #include <linux/nospec.h>
+#include <linux/audit.h>
 
 #define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PROG_ARRAY || \
 			   (map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
@@ -1011,6 +1012,34 @@ static void free_used_maps(struct bpf_prog_aux *aux)
 	kfree(aux->used_maps);
 }
 
+enum bpf_event {
+	BPF_EVENT_LOAD,
+	BPF_EVENT_UNLOAD,
+};
+
+static const char * const bpf_event_audit_str[] = {
+	[BPF_EVENT_LOAD]   = "LOAD",
+	[BPF_EVENT_UNLOAD] = "UNLOAD",
+};
+
+static void bpf_audit_prog(const struct bpf_prog *prog, enum bpf_event event)
+{
+	bool has_task_context = event == BPF_EVENT_LOAD;
+	struct audit_buffer *ab;
+
+	if (audit_enabled == AUDIT_OFF)
+		return;
+	ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_BPF);
+	if (unlikely(!ab))
+		return;
+	if (has_task_context)
+		audit_log_task(ab);
+	audit_log_format(ab, "%sprog-id=%u event=%s",
+			 has_task_context ? " " : "",
+			 prog->aux->id, bpf_event_audit_str[event]);
+	audit_log_end(ab);
+}
+
 int __bpf_prog_charge(struct user_struct *user, u32 pages)
 {
 	unsigned long memlock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
@@ -1112,6 +1141,7 @@ static void __bpf_prog_put_rcu(struct rcu_head *rcu)
 static void __bpf_prog_put(struct bpf_prog *prog, bool do_idr_lock)
 {
 	if (atomic_dec_and_test(&prog->aux->refcnt)) {
+		bpf_audit_prog(prog, BPF_EVENT_UNLOAD);
 		/* bpf_prog_free_id() must be called first */
 		bpf_prog_free_id(prog, do_idr_lock);
 		bpf_prog_kallsyms_del_all(prog);
@@ -1452,6 +1482,7 @@ static int bpf_prog_load(union bpf_attr *attr)
 	}
 
 	bpf_prog_kallsyms_add(prog);
+	bpf_audit_prog(prog, BPF_EVENT_LOAD);
 	return err;
 
 free_used_maps:
-- 
2.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 13:50 [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload Daniel Borkmann
@ 2018-10-04 17:11 ` Alexei Starovoitov
  2018-10-04 18:39   ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 16+ messages in thread
From: Alexei Starovoitov @ 2018-10-04 17:11 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: ast, netdev, brouer, Jiri Olsa, acme

On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
> Allow for audit messages to be emitted upon BPF program load and
> unload for having a timeline of events. The load itself is in
> syscall context, so additional info about the process initiating
> the BPF prog creation can be logged and later directly correlated
> to the unload event.
> 
> The only info really needed from BPF side is the globally unique
> prog ID where then audit user space tooling can query / dump all
> info needed about the specific BPF program right upon load event
> and enrich the record, thus these changes needed here can be kept
> small and non-intrusive to the core.

The above description is correct, but the commit log doesn't explain
_why_ this audit logging is needed and _why_ for load/unload.
My understanding of audit subsystem that it's very heavy weight
and absolutely not suitable for high frequency events.
Audit suppose to log the events that alter security of the system.
I don't see how loading/unloading bpf program influences security
at the time of the load.
The actions that program may take later (like dropping a packet
in XDP due to firewalling reasons) can be considered security
related, but not at the time of prog load.

Classic bpf for sockets and seccomp has been around for long time,
but seccomp audit messages don't trigger on bpf load/unload,
but rather on events like killing a process due to seccomp bpf return value.

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

Essentially we need perf binary to see bpf prog load/unload events with
single argument bpf_prog_id to be able to do its job.

I think from bpf kernel side there should be only one mechanism for user space
notifications and perf ring buffer fits the best, since amount
of load/unload in the system can be very large.
Anything but ring buffer will likely choke under volume of events.
Audit is not suitable for such notifications.

If in the future we have something other than seccomp killing
task via bpf return values, such code points would be good candidates
for audit logging.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 17:11 ` Alexei Starovoitov
@ 2018-10-04 18:39   ` Jesper Dangaard Brouer
  2018-10-04 19:41     ` Daniel Borkmann
  0 siblings, 1 reply; 16+ messages in thread
From: Jesper Dangaard Brouer @ 2018-10-04 18:39 UTC (permalink / raw)
  To: Alexei Starovoitov; +Cc: Daniel Borkmann, ast, netdev, Jiri Olsa, acme, brouer

On Thu, 4 Oct 2018 10:11:43 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:

> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
> > Allow for audit messages to be emitted upon BPF program load and
> > unload for having a timeline of events. The load itself is in
> > syscall context, so additional info about the process initiating
> > the BPF prog creation can be logged and later directly correlated
> > to the unload event.
> > 
> > The only info really needed from BPF side is the globally unique
> > prog ID where then audit user space tooling can query / dump all
> > info needed about the specific BPF program right upon load event
> > and enrich the record, thus these changes needed here can be kept
> > small and non-intrusive to the core.  
> 
> The above description is correct, but the commit log doesn't explain
> _why_ this audit logging is needed and _why_ for load/unload.
> My understanding of audit subsystem that it's very heavy weight
> and absolutely not suitable for high frequency events.
> Audit suppose to log the events that alter security of the system.
> I don't see how loading/unloading bpf program influences security
> at the time of the load.
> The actions that program may take later (like dropping a packet
> in XDP due to firewalling reasons) can be considered security
> related, but not at the time of prog load.

I actually consider loading (well attaching) an XDP program as a
security event.  With XDP you can bypass any kernel firewall and you are
invisible to tcpdump.  And without an audit event, you leave no record
of your actions.

This is not a thought up attack angle.  The NorthSec CTF hacker event,
had an XDP challenge that did just this.

Code: https://github.com/jdesfossez/nsec17-xdpbpf
Blog: https://suchakra.wordpress.com/2017/05/23/an-entertaining-ebpf-xdp-adventure/

 
> Classic bpf for sockets and seccomp has been around for long time,
> but seccomp audit messages don't trigger on bpf load/unload,
> but rather on events like killing a process due to seccomp bpf return value.
> 
> 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.
> 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/
> 
> Essentially we need perf binary to see bpf prog load/unload events with
> single argument bpf_prog_id to be able to do its job.
> 
> I think from bpf kernel side there should be only one mechanism for user space
> notifications and perf ring buffer fits the best, since amount
> of load/unload in the system can be very large.
> Anything but ring buffer will likely choke under volume of events.

I don't really buy the volume argument. Will the BPF-verifier not be
the bottleneck for the amount/volume of bpf_load events that can be
generated?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 18:39   ` Jesper Dangaard Brouer
@ 2018-10-04 19:41     ` Daniel Borkmann
  2018-10-04 20:22       ` Jesper Dangaard Brouer
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Borkmann @ 2018-10-04 19:41 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, Alexei Starovoitov; +Cc: ast, netdev, Jiri Olsa, acme

On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> On Thu, 4 Oct 2018 10:11:43 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>> On Thu, Oct 04, 2018 at 03:50:38PM +0200, Daniel Borkmann wrote:
>>> Allow for audit messages to be emitted upon BPF program load and
>>> unload for having a timeline of events. The load itself is in
>>> syscall context, so additional info about the process initiating
>>> the BPF prog creation can be logged and later directly correlated
>>> to the unload event.
>>>
>>> The only info really needed from BPF side is the globally unique
>>> prog ID where then audit user space tooling can query / dump all
>>> info needed about the specific BPF program right upon load event
>>> and enrich the record, thus these changes needed here can be kept
>>> small and non-intrusive to the core.  
>>
>> The above description is correct, but the commit log doesn't explain
>> _why_ this audit logging is needed and _why_ for load/unload.
>> My understanding of audit subsystem that it's very heavy weight
>> and absolutely not suitable for high frequency events.
>> Audit suppose to log the events that alter security of the system.
>> I don't see how loading/unloading bpf program influences security
>> at the time of the load.
>> The actions that program may take later (like dropping a packet
>> in XDP due to firewalling reasons) can be considered security
>> related, but not at the time of prog load.
> 
> I actually consider loading (well attaching) an XDP program as a
> security event.  With XDP you can bypass any kernel firewall and you are
> invisible to tcpdump.  And without an audit event, you leave no record
> of your actions.
> 
> This is not a thought up attack angle.  The NorthSec CTF hacker event,
> had an XDP challenge that did just this.
> 
> Code: https://github.com/jdesfossez/nsec17-xdpbpf
> Blog: https://suchakra.wordpress.com/2017/05/23/an-entertaining-ebpf-xdp-adventure/

Well, I don't think this argument is valid. First of all it is a
perfectly valid use case that XDP itself /is/ your firewall, and I
would presume it may even be the case or part of it in majority of
users of XDP, so saying bypassing any kernel firewall is not true.
Second, it offers all the introspection needed from iproute2 and
bpftool side to check what it is doing, and aside from that adding
to tc ingress is not a "security" event either that would need an
audit event.

>> Classic bpf for sockets and seccomp has been around for long time,
>> but seccomp audit messages don't trigger on bpf load/unload,
>> but rather on events like killing a process due to seccomp bpf return value.
>>
>> 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/

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.

>> Essentially we need perf binary to see bpf prog load/unload events with
>> single argument bpf_prog_id to be able to do its job.
>>
>> I think from bpf kernel side there should be only one mechanism for user space
>> notifications and perf ring buffer fits the best, since amount
>> of load/unload in the system can be very large.
>> Anything but ring buffer will likely choke under volume of events.
> 
> I don't really buy the volume argument. Will the BPF-verifier not be
> the bottleneck for the amount/volume of bpf_load events that can be
> generated?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 19:41     ` Daniel Borkmann
@ 2018-10-04 20:22       ` Jesper Dangaard Brouer
  2018-10-04 22:10         ` Alexei Starovoitov
  0 siblings, 1 reply; 16+ messages in thread
From: Jesper Dangaard Brouer @ 2018-10-04 20:22 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Alexei Starovoitov, ast, netdev, Jiri Olsa, acme, brouer

On Thu, 4 Oct 2018 21:41:17 +0200 Daniel Borkmann <daniel@iogearbox.net> wrote:

> On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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/  
> 
> 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.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 20:22       ` Jesper Dangaard Brouer
@ 2018-10-04 22:10         ` Alexei Starovoitov
  2018-10-05  6:14           ` Jiri Olsa
  2018-10-08 11:57           ` Jiri Olsa
  0 siblings, 2 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2018-10-04 22:10 UTC (permalink / raw)
  To: Jesper Dangaard Brouer; +Cc: Daniel Borkmann, ast, netdev, Jiri Olsa, acme

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 <daniel@iogearbox.net> wrote:
> 
> > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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/  
> > 
> > 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...
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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 22:10         ` Alexei Starovoitov
@ 2018-10-05  6:14           ` Jiri Olsa
  2018-10-05 18:44             ` Alexei Starovoitov
  2018-10-08 11:57           ` Jiri Olsa
  1 sibling, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2018-10-05  6:14 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa, acme

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 <daniel@iogearbox.net> wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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.
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-05  6:14           ` Jiri Olsa
@ 2018-10-05 18:44             ` Alexei Starovoitov
  2018-10-05 19:42               ` Arnaldo Carvalho de Melo
  2018-10-05 22:05               ` Jiri Olsa
  0 siblings, 2 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2018-10-05 18:44 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa, acme

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 <daniel@iogearbox.net> wrote:
> > > 
> > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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.

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-05 18:44             ` Alexei Starovoitov
@ 2018-10-05 19:42               ` Arnaldo Carvalho de Melo
  2018-10-05 20:26                 ` Alexei Starovoitov
  2018-10-05 22:05               ` Jiri Olsa
  1 sibling, 1 reply; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-10-05 19:42 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev,
	Jiri Olsa

Em Fri, Oct 05, 2018 at 11:44:35AM -0700, Alexei Starovoitov escreveu:
> 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:
> > > > 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.

I think Jiri just said that one would have to run something all the time
to get all the records, see below

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

I think he is talking about the preexisting loaded BPF programs. We have
the same problem with mmaps, where the perf tool will, with races,
enumerate the existing mmaps as PERF_RECORD_MMAP synthesized from
/proc/PIDS/smaps.

There was talk in the past to ask the kernel to emit PERF_RECORD_MMAP
into the ring buffer for those pre-existing entries, reducing a bit the
races, but as there doesn't seem to have a good way of doing it, we
continued with the synthesizing from procfs.

Is there a way for us to synthesize those prog load/unload for
preexisting loaded bpf objects?

- Arnaldo

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-05 19:42               ` Arnaldo Carvalho de Melo
@ 2018-10-05 20:26                 ` Alexei Starovoitov
  0 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2018-10-05 20:26 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev,
	Jiri Olsa

On Fri, Oct 05, 2018 at 04:42:49PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> Is there a way for us to synthesize those prog load/unload for
> preexisting loaded bpf objects?

see 'bpftool prog show'.
get_next_id + get_fd_by_id solve it race free.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-05 18:44             ` Alexei Starovoitov
  2018-10-05 19:42               ` Arnaldo Carvalho de Melo
@ 2018-10-05 22:05               ` Jiri Olsa
  2018-10-07 16:19                 ` Jesper Dangaard Brouer
  1 sibling, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2018-10-05 22:05 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa, acme

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 <daniel@iogearbox.net> wrote:
> > > > 
> > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-05 22:05               ` Jiri Olsa
@ 2018-10-07 16:19                 ` Jesper Dangaard Brouer
  2018-10-18 19:53                   ` Richard Guy Briggs
  0 siblings, 1 reply; 16+ messages in thread
From: Jesper Dangaard Brouer @ 2018-10-07 16:19 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Alexei Starovoitov, Daniel Borkmann, ast, netdev, Jiri Olsa,
	acme, brouer, Richard Guy Briggs

On Sat, 6 Oct 2018 00:05:22 +0200
Jiri Olsa <jolsa@redhat.com> wrote:

> 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 <daniel@iogearbox.net> wrote:
> > > > >   
> > > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:  
> > > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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/    
> > > 
[...]
> > > > > > 
> > > > > > 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 ;-)

Interesting, I also didn't consider teaching auditd to gets its 'bpf'
events from a separate perf ring-buffer, that might work.  I do wonder
how the audit people will take this suggestion.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-04 22:10         ` Alexei Starovoitov
  2018-10-05  6:14           ` Jiri Olsa
@ 2018-10-08 11:57           ` Jiri Olsa
  2018-10-10 19:53             ` Alexei Starovoitov
  1 sibling, 1 reply; 16+ messages in thread
From: Jiri Olsa @ 2018-10-08 11:57 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa, acme

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 <daniel@iogearbox.net> wrote:
> > 
> > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-08 11:57           ` Jiri Olsa
@ 2018-10-10 19:53             ` Alexei Starovoitov
  0 siblings, 0 replies; 16+ messages in thread
From: Alexei Starovoitov @ 2018-10-10 19:53 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jesper Dangaard Brouer, Daniel Borkmann, ast, netdev, Jiri Olsa, acme

On Mon, Oct 08, 2018 at 01:57:40PM +0200, Jiri Olsa wrote:
> 
> 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?

I agree that on the first glance ring buffer notifications
for program attachment sound useful, but I have a hard time
seeing how we can build the complete solution on top of them.
progs can be attached via netlink, perf_event ioctl, bpf syscall.
Theoretically we can insert ring buffer events in all those places,
but there is no common format we can use. In networking cases
ifindex alone won't be enough, since there is xdp vs tc vs lwt vs etc.
Furthermore the program can run without being attached to anything.
Like the way folks use xdp is they load mini bpf prog that dispatches
all other progs via prog_array via tail_call mechanism.
So to execute newly loaded program users space only needs to store its FD
into prog_array.
Would you want to add notifications for all map updates then as well?
What would be the format of such notification? map_id and slot index?
but how audit daemon will now that this particular map is used
for running and under what conditions?
Single bpf dispatcher program can use multiple prog_arrays.
It seems to me that attach notifications are not a practical way
to introspect the "bpf program execution graph" in the kernel.
I suggest to take a look at bpftool. It can inspect networking,
cgroup, tracing progs already and show what programs are loaded and where
they are attached to. I think improving bpftool style of introspection
is more practical than inventing notifications for everything.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-07 16:19                 ` Jesper Dangaard Brouer
@ 2018-10-18 19:53                   ` Richard Guy Briggs
  2018-10-18 22:09                     ` Steve Grubb
  0 siblings, 1 reply; 16+ messages in thread
From: Richard Guy Briggs @ 2018-10-18 19:53 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, ast, netdev,
	Jiri Olsa, acme, linux-audit

On 2018-10-07 18:19, Jesper Dangaard Brouer wrote:
> On Sat, 6 Oct 2018 00:05:22 +0200
> Jiri Olsa <jolsa@redhat.com> wrote:
> 
> > 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 <daniel@iogearbox.net> wrote:
> > > > > >   
> > > > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:  
> > > > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov <alexei.starovoitov@gmail.com> 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/    
> > > > 
> [...]
> > > > > > > 
> > > > > > > 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 ;-)
> 
> Interesting, I also didn't consider teaching auditd to gets its 'bpf'
> events from a separate perf ring-buffer, that might work.  I do wonder
> how the audit people will take this suggestion.

Including the linux-audit list to get userspace audit folks in the
loop...

> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer

- RGB

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload
  2018-10-18 19:53                   ` Richard Guy Briggs
@ 2018-10-18 22:09                     ` Steve Grubb
  0 siblings, 0 replies; 16+ messages in thread
From: Steve Grubb @ 2018-10-18 22:09 UTC (permalink / raw)
  To: linux-audit
  Cc: Richard Guy Briggs, Jesper Dangaard Brouer, Daniel Borkmann,
	netdev, ast, acme, Jiri Olsa, Jiri Olsa, Alexei Starovoitov

On Thursday, October 18, 2018 3:53:06 PM EDT Richard Guy Briggs wrote:
> On 2018-10-07 18:19, Jesper Dangaard Brouer wrote:
> > On Sat, 6 Oct 2018 00:05:22 +0200
> > 
> > Jiri Olsa <jolsa@redhat.com> wrote:
> > > 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 
<daniel@iogearbox.net> wrote:
> > > > > > > > On 10/04/2018 08:39 PM, Jesper Dangaard Brouer wrote:
> > > > > > > > > On Thu, 4 Oct 2018 10:11:43 -0700 Alexei Starovoitov 
<alexei.starovoitov@gmail.com> 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/
> > 
> > [...]
> > 
> > > > > > > > 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 ;-)
> > 
> > Interesting, I also didn't consider teaching auditd to gets its 'bpf'
> > events from a separate perf ring-buffer, that might work.  I do wonder
> > how the audit people will take this suggestion.

I'm not sure exactly what the issue is. You can audit for specific syscall 
and argument. So, if you want to see loads, then you can make a rule like:

-a always,exit -F arch=b64 -S bpf -F a0=5

-Steve

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-10-19  6:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 13:50 [PATCH bpf-next] bpf: emit audit messages upon successful prog load and unload Daniel Borkmann
2018-10-04 17:11 ` Alexei Starovoitov
2018-10-04 18:39   ` Jesper Dangaard Brouer
2018-10-04 19:41     ` Daniel Borkmann
2018-10-04 20:22       ` Jesper Dangaard Brouer
2018-10-04 22:10         ` Alexei Starovoitov
2018-10-05  6:14           ` Jiri Olsa
2018-10-05 18:44             ` Alexei Starovoitov
2018-10-05 19:42               ` Arnaldo Carvalho de Melo
2018-10-05 20:26                 ` Alexei Starovoitov
2018-10-05 22:05               ` Jiri Olsa
2018-10-07 16:19                 ` Jesper Dangaard Brouer
2018-10-18 19:53                   ` Richard Guy Briggs
2018-10-18 22:09                     ` Steve Grubb
2018-10-08 11:57           ` Jiri Olsa
2018-10-10 19:53             ` Alexei Starovoitov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.