Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing `rcu_read_lock()` or `xa_lock()` around `xa_for_each_marked()` iteration leads to a use-after-free on `group`. --
--- Patch [1]: [PATCH] drm/panthor: Display priorities of panthor groups over debugfs --- commit 55662349e547cf65d238c177ee7f9c17e8fd597c Author: Nicolas Frattaroli <[email protected]> drm/panthor: Display priorities of panthor groups over debugfs This commit adds debugfs information for panthor groups, showing the priority of each group. The implementation uses xa_for_each_marked to iterate through registered groups. > Some additional notes: I noticed panthor apparently uses > xa_for_each{_marked} by guarding it with xa_lock/xa_unlock. That appears > to be unnecessary, judging by the documentation of it and that nobody > else does this. Is this assumption accurate? While xa_for_each_marked() protects the XArray's internal state without external locks, the returned object pointers require either an rcu_read_lock() or xa_lock() around the entire loop to prevent them from being freed while they are being accessed inside the loop. > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > b/drivers/gpu/drm/panthor/panthor_sched.c > index 5832dccfc0932..44b61e946e2d4 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c [ ... ] > @@ -4198,3 +4199,91 @@ int panthor_sched_init(struct panthor_device *ptdev) [ ... ] > +static int show_file_groups(struct panthor_file *pfile, struct seq_file *m) > +{ > + struct panthor_group *group; > + unsigned long i; > + > + if (IS_ERR_OR_NULL(pfile->groups)) > + return -ENOENT; > + > + xa_for_each_marked(&pfile->groups->xa, i, group, GROUP_REGISTERED) { > + seq_printf(m, " Group %lu: priority %s\n", i, > + panthor_sched_prio_str(group->priority)); > + } [Severity: High] Does this code risk a use-after-free on the group pointer? Because xa_for_each_marked() only takes the RCU read lock internally during the lookup, the returned group pointer has no lifetime protection once the macro yields it to the loop body. If a user reads the sched_groups debugfs file concurrently with a group being destroyed via the DRM_IOCTL_PANTHOR_GROUP_DESTROY ioctl, the group could be removed and freed before its priority is accessed here. Should this iteration be protected by an rcu_read_lock() or xa_lock() to ensure the group pointer remains valid during the loop? > + > + return 0; > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260818-panthor-sched-group-prio-v1-1-1836857c5...@collabora.com?part=1
