Re: [PATCH] drm/panthor: Display priorities of panthor groups over debugfs

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-1836857c53e9@collabora.com?part=1