Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] bpf: PER_TASK map leak via premature sched_process_exit cleanup --
commit 9e1db08be07757261ff0e098cd3dda05c445d860 Author: Gabriele Monaco <[email protected]> tools/rv: Add BPF monitors This commit adds the code for two initial BPF monitors (queue and nohz) and adapts the in-kernel da_monitor.h to share code and conditionally compile BPF or kernel-specific components. It also integrates BPF monitor building into the main rv Makefile. > diff --git a/tools/verification/rv/bpf_monitors/da_monitor_bpf.h > b/tools/verification/rv/bpf_monitors/da_monitor_bpf.h > new file mode 100644 > index 0000000000000..12f0f78b9c9d3 > --- /dev/null > +++ b/tools/verification/rv/bpf_monitors/da_monitor_bpf.h [ ... ] > +#if RV_MON_TYPE == RV_MON_PER_TASK > +/* > + * BPF Per-Task monitor - uses BPF hash map indexed by PID > + * > + * Just a special case of per-object monitor with API consistent with > in-kernel > + * monitors (no need to pass the id). > + */ > + > +#define da_get_monitor(tsk) da_get_monitor_bpf(BPF_CORE_READ(tsk, pid), tsk) > +#define da_handle_event(tsk, event) \ > + da_handle_event_bpf(BPF_CORE_READ(tsk, pid), tsk, event) > +#define da_handle_start_event(tsk, event) \ > + da_handle_start_event_bpf(BPF_CORE_READ(tsk, pid), tsk, event) > +#define da_handle_start_run_event(tsk, event) \ > + da_handle_start_run_event_bpf(BPF_CORE_READ(tsk, pid), tsk, event) > + > +typedef struct task_struct *monitor_target_bpf; > +static inline void da_destroy_storage(da_id_type id); > + > +SEC("tp_btf/sched_process_exit") > +int BPF_PROG(handle_obj_cleanup, struct task_struct *p, bool group_dead) > +{ > + da_destroy_storage(p->pid); > + return 0; > +} [Severity: Medium] Could this lead to a map leak if trace events occur during task teardown after sched_process_exit? If an active PER_TASK monitor traces an event that occurs in do_exit() after sched_process_exit has already executed (for example, sched_dequeue, page faults, or other events in exit_mm() or schedule()), the event handler will call da_handle_start_event_bpf(). This function recreates the map element via da_create_storage(). Since handle_obj_cleanup has already run for this task, the newly recreated map element will never be deleted. Over time, this could exhaust the BPF map, and a future task reusing the same PID could inherit this stale automaton state. Would it be safer to attach this cleanup routine to sched_process_free instead, ensuring the storage is only destroyed when the task struct is actually being freed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
