On Mon, 10 Feb 2020 19:01:06 +0000 Emil Velikov emil.l.velikov@gmail.com wrote:
Thanks for having a look Daniel.
On Fri, 7 Feb 2020 at 13:29, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Feb 05, 2020 at 05:48:39PM +0000, Emil Velikov wrote:
From: Emil Velikov emil.velikov@collabora.com
This commit reworks the permission handling of the two ioctls. In particular it enforced the CAP_SYS_ADMIN check only, if:
- we're issuing the ioctl from process other than the one which opened
the node, and
- we are, or were master in the past
This allows for any application which cannot rely on systemd-logind being present (for whichever reason), to drop it's master capabilities (and regain them at later point) w/o being ran as root.
See the comment above drm_master_check_perm() for more details.
Cc: Adam Jackson ajax@redhat.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Emil Velikov emil.velikov@collabora.com
This effectively supersedes an earlier patch [1] incorporating ajax's feedback (from IRC ages ago).
[1] https://patchwork.freedesktop.org/patch/268977/
drivers/gpu/drm/drm_auth.c | 59 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_ioctl.c | 4 +-- include/drm/drm_file.h | 11 +++++++ 3 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index cc9acd986c68..01d9e35c0106 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c
+static int +drm_master_check_perm(struct drm_device *dev, struct drm_file *file_priv) +{
if (file_priv->pid != task_pid(current) && file_priv->was_master)Isn't this a typo? Why should we only allow this if the opener is someone else ... that looks like the logind approach? Or is my bolean logic parser broken again.
Thanks for spotting it. Indeed that should be:
if (file_priv->pid == task_pid(current) && file_priv->was_master) return 0;
Hi,
I'm mostly just curious, why is comparing pids safe here? Maybe the 'pid' member is not what userspace calls PID?
What if a malicious process receives a DRM fd from something similar to logind, then the logind equivalent process dies, and the malicious process starts forking new processes attempting to hit the same pid the logind equivalent had, succeeds in that, and passes the DRM fd to that fork. Is the fork then effectively in control of DRM master?
Thanks, pq