Re: [PATCH] bpf: restore the ebpf audit UNLOAD id field

On 12/21, Paul Moore wrote: > When changing the ebpf program put() routines to support being called > from within IRQ context the program ID was reset to zero prior to > generating the audit UNLOAD record, which obviously rendered the ID > field bogus (always zero). This patch resolves this by adding a new > field, bpf_prog_aux::id_audit, which is set when the ebpf program is > allocated an ID and never reset, ensuring a valid ID field, > regardless of the state of the original ID field, bpf_prox_aud::id. > I also modified the bpf_audit_prog() logic used to associate the > AUDIT_BPF record with other associated records, e.g. @ctx != NULL. > Instead of keying off the operation, it now keys off the execution > context, e.g. '!in_irg && !irqs_disabled()', which is much more > appropriate and should help better connect the UNLOAD operations with > the associated audit state (other audit records). [..] > As an note to future bug hunters, I did briefly consider removing the > ID reset in bpf_prog_free_id(), as it would seem that once the > program is removed from the idr pool it can no longer be found by its > ID value, but commit ad8ad79f4f60 ("bpf: offload: free program id > when device disappears") seems to imply that it is beneficial to > reset the ID value. Perhaps as a secondary indicator that the ebpf > program is unbound/orphaned. That seems like the way to go imho. Can we have some extra 'invalid_id' bitfield in the bpf_prog so we can set it in bpf_prog_free_id and check in bpf_prog_free_id (for this offloaded use-case)? Because having two ids and then keeping track about which one to use, depending on the context, seems more fragile?

> Fixes: d809e134be7a ("bpf: Prepare bpf_prog_put() to be called from irq > context.") > Reported-by: Burn Alting <burn.alting(a)iinet.net.au&gt; > Signed-off-by: Paul Moore <paul(a)paul-moore.com&gt; > --- > include/linux/bpf.h | 1 + > kernel/bpf/syscall.c | 8 +++++--- > 2 files changed, 6 insertions(+), 3 deletions(-)