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. > > > > 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. > > 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. > > 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 -- Simona Vetter Software Engineer http://blog.ffwll.ch
