On 9/9/26 12:21, Simona Vetter wrote:
> On Wed, Sep 09, 2026 at 01:00:16PM +0300, Joonas Lahtinen wrote:
>> (Adding bunch of FUSE folks and DRM folks in the hopes of coming to a
>> resolution here.)
>>
>> Quoting Joonas Lahtinen (2026-09-07 16:24:40)
>>> Quoting [email protected] (2026-09-03 18:22:23)
>>>> Thank you for your contribution! Sashiko AI review found 2 potential 
>>>> issue(s) to consider:
>>>> - [High] Global ordered workqueue stall via copy_from_user block while 
>>>> holding ioctl_lock.
>>>> - [Medium] Event sequence numbers (seqno) can be queued out-of-order in 
>>>> the event stream due to a race between seqno generation and event queuing.
>>>> --
>>>>
>>>> commit e2061e72d8e954a17bde35e9c970ee41bc2db0ad
>>>> Author: Mika Kuoppala <[email protected]>
>>>>
>>>>     drm/xe/eudebug: Introduce discovery for resources
>>>>
>>>> This commit introduces a resource discovery mechanism for debugger 
>>>> connections.
>>>> When a debugger connects, all existing resources for the target client are
>>>> relayed as events via an ordered workqueue, and DRM ioctls are blocked 
>>>> using
>>>> an rw_semaphore until discovery is complete.
>>>>
>>>>> diff --git a/drivers/gpu/drm/xe/xe_device.c 
>>>>> b/drivers/gpu/drm/xe/xe_device.c
>>>>> index 032a7ec5fbc8d..f10959b898297 100644
>>>>> --- a/drivers/gpu/drm/xe/xe_device.c
>>>>> +++ b/drivers/gpu/drm/xe/xe_device.c
>>>> [ ... ]
>>>>> @@ -238,8 +239,12 @@ static long xe_drm_ioctl(struct file *file, unsigned 
>>>>> int cmd, unsigned long arg)
>>>>>  
>>>>>       ACQUIRE(xe_pm_runtime_ioctl, pm)(xe);
>>>>>       ret = ACQUIRE_ERR(xe_pm_runtime_ioctl, &pm);
>>>>> -     if (ret >= 0)
>>>>> +     if (ret >= 0) {
>>>>> +             bool lock = xe_eudebug_discovery_lock(file, cmd);
>>>>>               ret = drm_ioctl(file, cmd, arg);
>>>>
>>>> [Severity: High]
>>>> Does holding the discovery lock across drm_ioctl() create a vector for a
>>>> workqueue stall? 
>>>>
>>>> If an unprivileged user maps the ioctl argument pointer to a blocking FUSE
>>>> or userfaultfd region, drm_ioctl() can be stalled indefinitely during
>>>> copy_from_user() while holding xef->eudebug.ioctl_lock for read.
>>>
>>> Don't think this is a very realistic vector to address, as it would
>>> also extend to every other copy_from_user() and also to userptr across
>>> all drivers.

Yeah I don't think that this is a major problem.

Using copy_from_user() while holding a lock is usually fundamentally broken in 
the first place.

But there are other issues which are much more problematic.

>>>
>>> Having a malfunctioning FUSE driver and getting a malfunctioning system
>>> as a result is probably somewhat expected.
>>
>> Based on further chatting on this with Sima, I was volunteered to pull
>> together the discussion here.
>>
>> We seem to have Sashiko picking up on patterns about accessing userspace
>> memory with locks held and potential for copy_from_user() (or userptr) to
>> then take indefinitely long to resolve. And that spreads to deadlocks
>> everywhere situation very fast.
>>
>> Based on reading of [1] and [2], it seems pretty much expected FUSE
>> drivers can trivially deadlock and ultimately in worst case the situation
>> can only be solved by manually aborting those connections by sysadmin.

The real problem comes with userfaultfd and the combination with HMM.

Drivers implementing HMM usually use a background workers to resolve 
recoverable page faults using the function hmm_range_fault().

If userfaultfd together with hmm_range_fault() can block those background 
workers indefinitely it can block other applications from using the HW without 
any sysadmin having any chance to figure out what is going on.

That is a classic local deny of service attack and I fear hmm_range_fault() 
needs something like a timeout to handle that.
>> It also seems (from the Sashiko comments) that by design, there's no
>> upper bound for how long an operation can take, so a bad FUSE driver
>> may stall for however long it sees fit to serve page-fault or in the
>> case of [3] it may decide to not actually populate the PTEs (or maybe
>> invalidate them immediately).
>>
>> Should we really be refactoring the whole kernel for the sake of
>> knowingly allowing potentially malicious userspace driver to idefinitely
>> stall or incorrectly resolve page faults? That'll be quite a lot of
>> complexity added to all the other drivers.

+1

Regards,
Christian.

>>
>> Or should there be more protections on FUSE / uffd to ensure such
>> idefinitive stall can't happen? Or maybe this is just an academic
>> problem and we amend review-prompts not to bring it up?
>>
>> Or maybe I missed some part of the FUSE docs and this isn't a real
>> problem?
> 
> Thanks for typing this up, matches what I think is going on here.
> 
>> Regards, Joonas
>>
>> PS. There is a related patch in [3] which tries to address the problem,
>> but we'll quickly run into live-locks and other issues even if we
>> refactored things into: pre-fault, take locks, do _nofault() access, and
>> retry if that fails.
> 
> Yeah just quickly wanting to add here that in my opinion, trying to sort
> this out in all the various subsystem is not how we should even start to
> think about this issue. This would be a fundamental change in how
> subsystems are allowed to nest locking with stuff that can trigger
> userspace faults.
> 
> I did ponder a bit how this could be solved on the fuse side of things,
> maybe with some seccomp style filters. Like maybe lockdep could be
> enlisted to help catch deadlocks, with a special "this is a fuse process,
> it all defacto runs in fault handler context. But that only catches bugs
> in normal use, not malicious exploits. And given that userspace can choose
> the timing and unblock at will (I think so at least), this is pretty
> powerful tool for being nasty to the kernel.
> 
> But mostly I want to really, really stand back in awe about this issue and
> not think too hard about it.
> 
> Cheers, Sima
> 
>> [1] 
>> https://www.kernel.org/doc/html/next/filesystems/fuse.html#kernel-userspace-interface
>> [2] 
>> https://www.kernel.org/doc/html/next/filesystems/fuse.html#aborting-a-filesystem-connection
>> [3] 
>> https://sashiko.dev/#/patchset/20260827062142.4038272-1-srinivasan.shanmugam%40amd.com
> 

Reply via email to