Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] rv_is_container_monitor() fails to identify an empty container if it is the last element in the list, leading to a NULL pointer dereference when enabled. -- commit a8e1e1f414fc69c200ed8ba3fee4d10346fe86df Author: Gabriele Monaco <[email protected]> rv: Add rv_get_monitor_by_name() Add a helper function to get the rv_monitor pointer for a registered monitor given its name and add its first user in rv_register_monitor(). > diff --git a/kernel/trace/rv/rv.c b/kernel/trace/rv/rv.c > index f77c5865f41ab..33919c5fbd6ad 100644 > --- a/kernel/trace/rv/rv.c > +++ b/kernel/trace/rv/rv.c [ ... ] > @@ -229,6 +230,18 @@ bool rv_is_container_monitor(struct rv_monitor *mon) > return next->parent == mon || !mon->enable; > } [Severity: High] This isn't a bug introduced by this patch, but there is a pre-existing issue in rv_is_container_monitor(). Does this function incorrectly classify an empty container monitor if it happens to be the last element in rv_monitors_list? Looking at the rest of rv_is_container_monitor(): bool rv_is_container_monitor(struct rv_monitor *mon) { ... if (list_is_last(&mon->list, &rv_monitors_list)) return false; next = list_next_entry(mon, list); return next->parent == mon || !mon->enable; } If an empty container monitor (which has a NULL enable function) is the last element, it returns false early. This bypasses the !mon->enable check, treating the empty container as a regular monitor. If a user then writes to its tracefs enable file, rv_enable_single() appears to unconditionally call the enable function: retval = mon->enable(); Since the empty container monitor has a NULL enable pointer, will this lead to a kernel panic via a NULL pointer dereference when enabled? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
