On 12/15/23 6:38 AM, Dave Tucker wrote:
Current output from auditd is as follows:
time->Wed Dec 13 21:39:24 2023
type=BPF msg=audit(1702503564.519:11241): prog-id=439 op=LOAD
This only tells you that a BPF program was loaded, but without
any context. If we include the prog-name, pid, uid and comm we get
output as follows:
time->Wed Dec 13 21:59:59 2023
type=BPF msg=audit(1702504799.156:99528): op=UNLOAD prog-id=50092
prog-name="test" pid=27279 uid=0 comm="new_name"
With pid, uid a system administrator has much better context
over which processes and user loaded which eBPF programs.
comm is useful since processes may be short-lived.
Signed-off-by: Dave Tucker <dave@xxxxxxxxxxxxx>
---
Changes:
v1->v2:
- Move 'op' to the front of the audit messages
- Add 'prog-name' to the audit messages
- Replace deprecated in_irq() with in_hardirq()
- Remove in_irq() check from bpf_audit_prog since it's always called
from the task context
Is this true? For condition '(in_hardirq() || irqs_disabled())
', the context will be the task context. But what about nmi and
softirq? The context maynot be the task context, right?
Not sure whether this is relevant or not. There was a discussion
in the past about the above condition. See
https://lore.kernel.org/bpf/a93079f2-fcd4-e3ef-3b92-92d443b8e8c6@xxxxxxxx/
The recommended change was
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index
a75c54b6f8a3..11df562e481b 100644 --- a/kernel/bpf/syscall.c +++
b/kernel/bpf/syscall.c @@ -2147,7 +2147,7 @@ static void
__bpf_prog_put(struct bpf_prog *prog) struct bpf_prog_aux *aux = prog->aux;
if (atomic64_dec_and_test(&aux->refcnt)) {
- if (in_irq() || irqs_disabled()) { + if (!in_interrupt()) { INIT_WORK(&aux->work, bpf_prog_put_deferred);
schedule_work(&aux->work);
} else {
If the above change was true, then audit will not be able to
get any meaning task specific information, you might need to
gather such informaiton before hand.
- Only populate pid, uid and comm if not in a kthread
kernel/bpf/syscall.c | 27 ++++++++++++++++++++++-----
1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 06320d9abf33..fbbaf3b8ad48 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -35,6 +35,7 @@
#include <linux/rcupdate_trace.h>
#include <linux/memcontrol.h>
#include <linux/trace_events.h>
+#include <linux/uidgid.h>
#include <net/netfilter/nf_bpf_link.h>
#include <net/netkit.h>
@@ -2110,18 +2111,34 @@ static void bpf_audit_prog(const struct bpf_prog *prog, unsigned int op)
{
struct audit_context *ctx = NULL;
struct audit_buffer *ab;
+ const struct cred *cred;
+ char comm[sizeof(current->comm)];
if (WARN_ON_ONCE(op >= BPF_AUDIT_MAX))
return;
if (audit_enabled == AUDIT_OFF)
return;
- if (!in_irq() && !irqs_disabled())
- ctx = audit_context();
+
+ ctx = audit_context();
ab = audit_log_start(ctx, GFP_ATOMIC, AUDIT_BPF);
if (unlikely(!ab))
return;
- audit_log_format(ab, "prog-id=%u op=%s",
- prog->aux->id, bpf_audit_str[op]);
+
+ audit_log_format(ab, "op=%s prog-id=%u",
+ bpf_audit_str[op], prog->aux->id);
+ audit_log_format(ab, " prog-name=");
+ audit_log_untrustedstring(ab, prog->aux->name ?: "(none)");
+
+ if (current->mm) {
+ cred = current_cred();
+ audit_log_format(ab, " pid=%u uid=%u",
+ task_pid_nr(current),
+ from_kuid(&init_user_ns, cred->uid));
+ audit_log_format(ab, " comm=");
+ audit_log_untrustedstring(ab, get_task_comm(comm, current));
+ } else {
+ audit_log_format(ab, " pid=? uid=? comm=?");
+ }
audit_log_end(ab);
}
@@ -2212,7 +2229,7 @@ static void __bpf_prog_put(struct bpf_prog *prog)
struct bpf_prog_aux *aux = prog->aux;
if (atomic64_dec_and_test(&aux->refcnt)) {
- if (in_irq() || irqs_disabled()) {
+ if (in_hardirq() || irqs_disabled()) {
INIT_WORK(&aux->work, bpf_prog_put_deferred);
schedule_work(&aux->work);
} else {
