On 2019/05/27, Emil Velikov wrote: > From: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> > > Currently one can circumvent DRM_AUTH, when the ioctl is exposed via the > render node. A seemingly deliberate design decision. > > Hence we can drop the DRM_AUTH all together (details in follow-up patch) > yet not all userspace checks if it's authenticated, but instead uses > uncommon assumptions. > > After days of digging through git log and testing, only a single (ab)use > was spotted - the Mesa RADV driver, using the AMDGPU_INFO ioctl and > assuming that failure implies lack of authentication. > > Affected versions are: > - the whole 18.2.x series, which is EOL > - the whole 18.3.x series, which is EOL > - the 19.0.x series, prior to 19.0.4 > > Add a special quirk for that case, thus we can drop DRM_AUTH bits as > mentioned earlier. > > Since all the affected userspace is EOL, we also add a kconfig option > to disable this quirk. > > The whole approach is inspired by DRIVER_KMS_LEGACY_CONTEXT > > Cc: Alex Deucher <alexander.deucher@xxxxxxx> > Cc: Christian König <christian.koenig@xxxxxxx> > Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx > Cc: David Airlie <airlied@xxxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Signed-off-by: Emil Velikov <emil.velikov@xxxxxxxxxxxxx> > --- > drivers/gpu/drm/amd/amdgpu/Kconfig | 16 ++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 12 +++++++++++- > drivers/gpu/drm/drm_ioctl.c | 5 +++++ > include/drm/drm_ioctl.h | 17 +++++++++++++++++ > 4 files changed, 49 insertions(+), 1 deletion(-) > Hi Christian, In the following, I would like to summarise and emphasize the need for DRM_AUTH removal. I would kindly ask you to spend a couple of minutes extra reading it. Today DRM drivers* do not make any distinction between primary and render node clients. Thus for a render capable driver, any premise of separation, security or otherwise imposed via DRM_AUTH is a fallacy. Considering the examples of flaky or outright missing drmAuth in prime open-source projects (mesa, kmscube, libva) we can reasonably assume other projects exbibit the same problem. For these and/or other reasons, two DRM drivers have dropped DRM_AUTH since day one. Since we are interested in providing consistent and predictable behaviour to user-space, dropping DRM_AUTH seems to be the most effective way forward. Of course, I'd be more than happy to hear for any other way to achieve the goal outlined. Based on the series, other maintainers agree with the idea/goal here. Amdgpu not following the same pattern would compromise predictability across drivers and complicate userspace, so I would kindly ask you to reconsider. Alternatively, I see two ways to special case: - New flag annotating amdgpu/radeon - akin to the one proposed earlier - Check for amdgpu/radeon in core DRM Now on your pain point - not allowing render iocts via the primary node, and how this patch could make it harder to achieve that goal. While I love the idea, there are number of obstacles that prevent us from doing so at this time: - Ensuring both old and new userspace does not regress - Drivers (QXL, others?) do not expose a render node - We want to avoid driver specific behaviour The only way forward that I can see is: - Address QXL/others to expose render nodes - Add a Kconfig toggle to disable !KMS ioctls via the primary node - Jump-start ^^ with distribution X - Fix user-space fallouts - X months down the line, flip the Kconfig - In case of regressions - revert the KConfig, goto Fix user-space... That said, the proposal will not conflict with the DRM_AUTH removal. If anything it is step 0.5 of the grand master plan. Tl;Dr: Removing DRM_AUTH is orthogonal to not allowing render iocts via the primary node. Here's an idea how to achieve the latter. Thanks Emil _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx
