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

[bot+bpf-ci at kernel.org]

> 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