Re: [PATCH RFC bpf-next 0/4] audit: Expose audit subsystem to BPF LSM programs via BPF kfuncs
Frederick Lawler <[email protected]> Wed, 18 Mar 2026 12:34:17 -0500
| Newsgroups | org.kernel.vger.audit,org.kernel.vger.bpf,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.linux-security-module |
|---|---|
| Message-ID | <abriGVbtYY1zhupD@CMGLRV3> |
Hi Kumar, On Tue, Mar 17, 2026 at 03:43:36AM +0100, Kumar Kartikeya Dwivedi wrote: > On Wed, 11 Mar 2026 at 22:31, Frederick Lawler <[email protected]> wrote: > > 3. All bpf_audit_log_*() functions are destructive > > > > The audit subsystem allows for AUDIT_FAIL_PANIC to be set when the > > subsystem can detect that missing events. Further, some call paths may > > invoke a BUG_ON(). Therefore all the functions are marked destructive. > > I think the first part makes sense (i.e., the policy simply configured > the system to panic on failure). > However, in the second case, if the program is somehow able to trigger > BUG_ON() relied upon for internal invariants, it would be considered > broken. > I tried grepping through and didn't find anything that would cause > this, hence the whole thing about BUG_ON() in the cover letter only > adds to confusion. > Please drop it or describe cases which you were concerned about. > bpf_audit_log_cause() -> audit_log_untrustedstring() -> audit_log_n_untrustedstring() -> audit_log_n_hex() Was the primary call chain I was referring to, and yes this relies on internal invariant of the SKB existing. I can remove from cover. > > > > 4. Functions are callable once per bpf_audit_context > > > > The rationale for this was to prevent abuse. Logs with repeated fields > > are not helpful, and may not be handled by user space audit coherently. > > > > This rationale feels weak. What abuse are we talking about? > The LSM program is already written by a trusted entity. > I learned through off-list discussions that there's "undocumented" or "unspoken" rules of log formatting and how auditd expects logs. Without knowing exactly what these rules are, I can't provide any stronger arguments other than to reduce duplication of fields in messages, or exclude wrappers that supply unverified data. WRT to unverified data, the socket wrappers accepting a netinf int. In that case, the variable is at least verified via lookup to report the interface in which it belongs. I left out wrapping LSM_AUDIT_DATA_ANONINODE due to the passing of arbitrary string, LSM_AUDIT_DATA_IPC arbitrary int. Others like for infiniband, lockdown, nlmsgtype, were left out due to specificity, and not general purpose usage. Some of the rationale behind this is _not_ trusting BPF LSM authors as much due to BPF LSM flexibility and accommodating user space audit. Best, Fred