Re: [PATCH RFC bpf-next 1/4] audit: Implement bpf_audit_log_*() wrappers

bot+bpf-ci
2026-03-11T22:08:19Z

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

> diff --git a/security/lsm_audit_kfuncs.c b/security/lsm_audit_kfuncs.c
> --- /dev/null
> +++ b/security/lsm_audit_kfuncs.c

[ ... ]

> +__bpf_kfunc
> +struct bpf_audit_context *bpf_audit_log_start(struct bpf_prog_aux *aux)
> +{
> + char comm[sizeof(current->comm)];
> + struct bpf_audit_context *ac;
> +
> + ac = bpf_mem_cache_alloc(&bpf_audit_context_ma);
> + if (!ac)
> + return NULL;
> +
> + memset(ac, 0, sizeof(*ac));
> + ac->ab = audit_log_start(audit_context(),
> + (aux->might_sleep) ? GFP_KERNEL : GFP_ATOMIC,
> + AUDIT_BPF_LSM_ACCESS);
> + if (!ac->ab) {
> + bpf_mem_cache_free(&bpf_audit_context_ma, ac);
> + return NULL;
> + }
> +
> + audit_log_format(ac->ab, "prog-id=%d", aux->id);

aux->id is u32 -- should this be %u?

> +
> + /* Audit may not have a filter configured for syscalls. Include
> + * potentionally redundant pid & comm information

Minor typo: "potentionally" -> "potentially".

> + */

[ ... ]

> +__bpf_kfunc int bpf_audit_log_net_sock(struct bpf_audit_context *ac, int netif,
> + const struct socket *sock)
> +{
> + struct lsm_network_audit net = { .sk = sock->sk, .netif = netif };
> + struct common_audit_data ad;
> +
> + ad.type = LSM_AUDIT_DATA_NET;
> + ad.u.net = &net;
> + return __audit_log_lsm_data(ac, &ad);
> +}
> +
> +__bpf_kfunc int
> +bpf_audit_log_net_sockaddr(struct bpf_audit_context *ac, int netif,
> + const struct sockaddr *saddr__nullable,
> + const struct sockaddr *daddr__nullable, int addrlen)
> +{
> + struct lsm_network_audit net;
^^^

Unlike bpf_audit_log_net_sock() above which uses a designated
initializer that zero-fills the struct, this declaration leaves
net uninitialized.

In particular, net.sk is never assigned anywhere in this function.
When audit_log_lsm_data() handles LSM_AUDIT_DATA_NET, it does:

if (a->u.net->sk) {
const struct sock *sk = a->u.net->sk;
switch (sk->sk_family) {

Since net.sk contains stack garbage, this will likely dereference
an invalid pointer and crash.

Additionally, when only one of saddr__nullable or daddr__nullable
is provided, the corresponding sport/dport and address fields for
the missing side remain uninitialized, and audit_log_lsm_data()
unconditionally prints both sides:

print_ipv4_addr(ab, a->u.net->v4info.saddr, a->u.net->sport, ...);
print_ipv4_addr(ab, a->u.net->v4info.daddr, a->u.net->dport, ...);

Would initializing with something like
struct lsm_network_audit net = { .netif = netif }; address both
issues?

> + struct common_audit_data ad;
> +
> + net.netif = netif;

[ ... ]

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22976062351