Re: [PATCH v10 11/27] drm/xe/eudebug: Add ufence events with acks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A race condition in `xe_eudebug_ufence_track` during debugger detachment leaves `ufence->eudebug.bind_ref_seqno` uncleared, triggering a `XE_WARN_ON` during ufence destruction.
- [High] Unbounded non-accounted kernel memory allocation in `xe_eudebug_track_ufence` allows a malicious debugger to exhaust system memory (DoS).
--

commit a5714f9c14ecadc50ce9b02c008c2b316e1293f6
Author: Mika Kuoppala <mika.kuoppala@linux.intel.com>

drm/xe/eudebug: Add ufence events with acks

This commit introduces ufence event tracking for the Xe eudebug interface.
It allows the debugger to intercept workloads and stall the delivery of
ufence signals until the debugger explicitly acknowledges the ufence via a
new ioctl.

> diff --git a/drivers/gpu/drm/xe/xe_eudebug.c b/drivers/gpu/drm/xe/xe_eudebug.c
> index fe3b2ae06a31f..a1f07db6ad817 100644
> --- a/drivers/gpu/drm/xe/xe_eudebug.c
> +++ b/drivers/gpu/drm/xe/xe_eudebug.c
[ ... ]
> @@ -1038,11 +1155,169 @@ static int vm_bind_op(struct xe_eudebug *d, struct xe_vm *vm,
[ ... ]
> +void xe_eudebug_ufence_fini(struct xe_user_fence *ufence)
> +{
> +	struct xe_eudebug *d;
> +
> +	XE_WARN_ON(READ_ONCE(ufence->eudebug.bind_ref_seqno));
> +
> +	d = READ_ONCE(ufence->eudebug.debugger);
> +	if (!d)
> +		return;
> +
> +	/* last ref */
> +	ufence->eudebug.debugger = NULL;
> +
> +	xe_eudebug_put(d);
> +}
> +
> +static int xe_eudebug_track_ufence(struct xe_eudebug *d,
> +				   struct xe_user_fence *f,
> +				   u64 seqno)
> +{
> +	struct xe_eudebug_ack *ack;
> +	struct rb_node *old;
> +
> +	ack = kzalloc_obj(*ack, GFP_KERNEL);
> +	if (!ack) {
> +		eu_dbg(d, "ack allocation fail");
> +		return -ENOMEM;
> +	}

[Severity: High]
Can this unbounded allocation be exploited by a malicious debugger to exhaust
kernel memory?

The allocation here uses GFP_KERNEL without __GFP_ACCOUNT (which would charge
it to the memory cgroup). A debugger could continuously read events via
DRM_XE_EUDEBUG_IOCTL_READ_EVENT to clear the FIFO, but never acknowledge them
using DRM_XE_EUDEBUG_IOCTL_ACK_EVENT.

Because the ACKs and their associated ufences are only freed when explicitly
acknowledged by userspace, a target process looping VM_BINDs with ufences
could cause d->acks.tree to grow indefinitely until a system-wide
Out-Of-Memory (OOM) panic occurs.

> +
> +	ack->seqno = seqno;
> +	ack->ts_insert = ktime_get();
[ ... ]
> +bool xe_eudebug_ufence_track(struct xe_user_fence *ufence)
> +{
> +	struct xe_eudebug *d;
> +	bool tracked = false;
> +	int ret;
> +
> +	spin_lock(&ufence->eudebug.lock);
> +	d = ufence->eudebug.debugger;
> +	if (d && !kref_get_unless_zero(&d->ref))
> +		d = NULL;
> +
> +	if (!d || xe_eudebug_detached(d)) {
> +		ufence->eudebug.bind_ref_seqno = 0;
> +		ufence->eudebug.signalled_seqno = 0;
> +	}
> +	spin_unlock(&ufence->eudebug.lock);
> +
> +	if (!d)
> +		return false;
> +
> +	if (xe_eudebug_detached(d)) {
> +		xe_eudebug_put(d);
> +		return false;
> +	}

[Severity: High]
Is there a race condition here if the debugger detaches right after the lock
is released?

If xe_eudebug_detached(d) becomes true immediately after
spin_unlock(&ufence->eudebug.lock), this check will evaluate to true and
return false without clearing ufence->eudebug.bind_ref_seqno and
signalled_seqno.

Since false is returned, the caller user_fence_worker() will signal and
release the ufence. When the ufence is subsequently destroyed,
xe_eudebug_ufence_fini() will hit the
XE_WARN_ON(READ_ONCE(ufence->eudebug.bind_ref_seqno)) assertion because the
field was never cleared.

> +
> +	ret = track_ufence(d, ufence, &tracked);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903145952.848051-1-mika.kuoppala@linux.intel.com?part=11