From patchwork Thu Mar 19 17:29:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emil Velikov X-Patchwork-Id: 11447741 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A2F676CA for ; Thu, 19 Mar 2020 17:30:38 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 777912071C for ; Thu, 19 Mar 2020 17:30:38 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mO11wYTv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 777912071C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E7F966EA41; Thu, 19 Mar 2020 17:30:34 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wr1-x444.google.com (mail-wr1-x444.google.com [IPv6:2a00:1450:4864:20::444]) by gabe.freedesktop.org (Postfix) with ESMTPS id 106296EA41 for ; Thu, 19 Mar 2020 17:30:34 +0000 (UTC) Received: by mail-wr1-x444.google.com with SMTP id w10so4164413wrm.4 for ; Thu, 19 Mar 2020 10:30:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=0d7zIwG53qE4nDg8R9G9mpn3/riI4mVlhLQJEQbgbHo=; b=mO11wYTvc9zHnfEmEDo/eozB2cxn5oC++d1dqEBUDeV4qd+T4D9MA0B4NByjtPIXdM 5ZL3uzwwWS+5nshyBoYmldgin1tRMoWWsnDxK5vOOExZRRFHjBkuOgT7+ZFWfF4MkxMg sFvoSqRn37D2+ICnV3kd+G8F5unhpWRxncyfRFuCteXT7Xp302lBRm+wYs9B9gR6RRoX 2PPSBR5S4SVrTNDWTBdqzYACcTbJHBd7VEgu4EHm/z8x4zcElXgIGoyY+gPw/+NhJ17t X1jUeFklGVk+th3CHv1E4fWDyrqx378mcVXN48wgvMsn6PkzOaYok15iZlL0eH2oA/l1 zm1g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=0d7zIwG53qE4nDg8R9G9mpn3/riI4mVlhLQJEQbgbHo=; b=VOibtvQE9jbo+LiWTBkDSzmq2SYT+8V0Cfd3kNYEan7d5MQUMI09eN3ra4KktPJ1Xk 2fw1MGAetaKWvwTzWIarPSpUiRlkMhYatPlp3uhyNDiG/nujtoDlnR4xs8VgIW0itoEq rr/75ZSGMvynoxCXmYY6fDIlezZlV4Mf9nlCnJujbpQmPOsKivNxDixMC7HlCp2NkW6t nQ4dj6JVv8pwR/g/9lWfWMJeildLDUSwQC0GDK1lN3pLrEKIgcKbvba1D3pkMi1WifM6 ld3xyFDqepJHb/XcZ5C0MTibXucvjLcuq9B2jbPHngNZz1sTWRYHRl/NwfFtIn9nL4gu 1YnA== X-Gm-Message-State: ANhLgQ0z/cvLUXI4DolSnLKMGSYKK6WAM6tLEfSSTSspdBdKhftzkici QHMHn2Vk5HGCL32E9eIzI9QCpVFR X-Google-Smtp-Source: ADFU+vtcajybqmS91n+pWUnHeO7uxluzI2IBvA5hwvrmLA+EMy9wYSjBONb0ufh2d/X8CSM0icdh/Q== X-Received: by 2002:adf:d4ca:: with SMTP id w10mr5508099wrk.407.1584639032217; Thu, 19 Mar 2020 10:30:32 -0700 (PDT) Received: from arch-x1c3.cbg.collabora.co.uk ([2a00:5f00:102:0:9665:9cff:feee:aa4d]) by smtp.gmail.com with ESMTPSA id k9sm4534601wrd.74.2020.03.19.10.30.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Mar 2020 10:30:31 -0700 (PDT) From: Emil Velikov To: dri-devel@lists.freedesktop.org Subject: [PATCH v2 1/2] drm: rework SET_MASTER and DROP_MASTER perm handling Date: Thu, 19 Mar 2020 17:29:29 +0000 Message-Id: <20200319172930.230583-1-emil.l.velikov@gmail.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emil.l.velikov@gmail.com, Daniel Vetter Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Emil Velikov 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 ensures that we: - do not regress the systemd-logind style of DRM_MASTER arbitrator - allow applications which do not use systemd-logind to drop their master capabilities (and regain them at later point) ... w/o running as root. See the comment above drm_master_check_perm() for more details. v1: - Tweak wording, fixup all checks, add igt test v2: - Add a few more comments, grammar nitpicks. Cc: Adam Jackson Cc: Daniel Vetter Cc: Pekka Paalanen Testcase: igt/core_setmaster/master-drop-set-user Signed-off-by: Emil Velikov Reviewed-by: Adam Jackson --- drivers/gpu/drm/drm_auth.c | 67 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_ioctl.c | 4 +-- include/drm/drm_file.h | 11 ++++++ 3 files changed, 80 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index cc9acd986c68..37cac0a221ff 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -135,6 +135,7 @@ static int drm_set_master(struct drm_device *dev, struct drm_file *fpriv, } } + fpriv->was_master = (ret == 0); return ret; } @@ -179,12 +180,72 @@ static int drm_new_set_master(struct drm_device *dev, struct drm_file *fpriv) return ret; } +/* + * In the olden days the SET/DROP_MASTER ioctls used to return EACCES when + * CAP_SYS_ADMIN was not set. This was used to prevent rogue applications + * from becoming master and/or failing to release it. + * + * At the same time, the first client (for a given VT) is _always_ master. + * Thus in order for the ioctls to succeed, one had to _explicitly_ run the + * application as root or flip the setuid bit. + * + * If the CAP_SYS_ADMIN was missing, no other client could become master... + * EVER :-( Leading to a) the graphics session dying badly or b) a completely + * locked session. + * + * + * As some point systemd-logind was introduced to orchestrate and delegate + * master as applicable. It does so by opening the fd and passing it to users + * while in itself logind a) does the set/drop master per users' request and + * b) * implicitly drops master on VT switch. + * + * Even though logind looks like the future, there are a few issues: + * - some platforms don't have equivalent (Android, CrOS, some BSDs) so + * root is required _solely_ for SET/DROP MASTER. + * - applications may not be updated to use it, + * - any client which fails to drop master* can DoS the application using + * logind, to a varying degree. + * + * * Either due missing CAP_SYS_ADMIN or simply not calling DROP_MASTER. + * + * + * Here we implement the next best thing: + * - ensure the logind style of fd passing works unchanged, and + * - allow a client to drop/set master, iff it is/was master at a given point + * in time. + * + * Note: DROP_MASTER cannot be free for all, as an arbitrator user could: + * - DoS/crash the arbitrator - details would be implementation specific + * - open the node, become master implicitly and cause issues + * + * As a result this fixes the following when using root-less build w/o logind + * - startx + * - weston + * - various compositors based on wlroots + */ +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) + return 0; + + if (!capable(CAP_SYS_ADMIN)) + return -EACCES; + + return 0; +} + int drm_setmaster_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { int ret = 0; mutex_lock(&dev->master_mutex); + + ret = drm_master_check_perm(dev, file_priv); + if (ret) + goto out_unlock; + if (drm_is_current_master(file_priv)) goto out_unlock; @@ -229,6 +290,12 @@ int drm_dropmaster_ioctl(struct drm_device *dev, void *data, int ret = -EINVAL; mutex_lock(&dev->master_mutex); + + ret = drm_master_check_perm(dev, file_priv); + if (ret) + goto out_unlock; + + ret = -EINVAL; if (!drm_is_current_master(file_priv)) goto out_unlock; diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 9e41972c4bbc..73e31dd4e442 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -599,8 +599,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_SET_SAREA_CTX, drm_legacy_setsareactx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_GET_SAREA_CTX, drm_legacy_getsareactx, DRM_AUTH), - DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, DRM_ROOT_ONLY), - DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, DRM_ROOT_ONLY), + DRM_IOCTL_DEF(DRM_IOCTL_SET_MASTER, drm_setmaster_ioctl, 0), + DRM_IOCTL_DEF(DRM_IOCTL_DROP_MASTER, drm_dropmaster_ioctl, 0), DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_ADD_CTX, drm_legacy_addctx, DRM_AUTH|DRM_ROOT_ONLY), DRM_LEGACY_IOCTL_DEF(DRM_IOCTL_RM_CTX, drm_legacy_rmctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY), diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 19df8028a6c4..c4746c9d3619 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -201,6 +201,17 @@ struct drm_file { */ bool writeback_connectors; + /** + * @was_master: + * + * This client has or had, master capability. Protected by struct + * &drm_device.master_mutex. + * + * This is used to ensure that CAP_SYS_ADMIN is not enforced, if the + * client is or was master in the past. + */ + bool was_master; + /** * @is_master: * From patchwork Thu Mar 19 17:29:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Emil Velikov X-Patchwork-Id: 11447743 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9336A6CA for ; Thu, 19 Mar 2020 17:30:41 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6F2502071C for ; Thu, 19 Mar 2020 17:30:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="gcv5pJM3" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6F2502071C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 7EA966EA42; Thu, 19 Mar 2020 17:30:37 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) by gabe.freedesktop.org (Postfix) with ESMTPS id E45F36EA42 for ; Thu, 19 Mar 2020 17:30:35 +0000 (UTC) Received: by mail-wm1-x343.google.com with SMTP id z12so3305539wmf.5 for ; Thu, 19 Mar 2020 10:30:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=KdVK0dOTtxsCHG3yFkwkSQdVvO5AfUmYKEhbPilZuP8=; b=gcv5pJM3gYdQWuSZbOmAnHyr2wpaiBPs0UiZAkaZNB5z99ED7ueH2h1gWIhoWT8GKW lG1nwZzlroE2zv+VbEQaH3ip6+XJNwGi0tRL4fjYJP3NbRTLywkeENa93L7ifANNTiEI biR1CPjKZaHKEgfoVDFqZZjeXdfLoPeGWqbXJPVXZth5cpbiQ2hfF2g/YzyRGURLg5/4 GVnsA9N8H2bnO+YauycekTlrcTDxHhOO3HOU5bDRz7Z2Mg4p+u2SuLF7NuspS6T8awhR fD05d5/AIWm2MQeD9pOh+WnK65fMnTIdBpSMAmAqpKTe59daQP0GQ3rUsoczejd6J9u5 n+2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=KdVK0dOTtxsCHG3yFkwkSQdVvO5AfUmYKEhbPilZuP8=; b=YXc5j2s/PV0xYFpMCPSg3a9XDshabx/kFObsPLTMjztYHcH2+i/mEh511T5TshFADM baOTOs6Y8MqJjBzj8VUxBAqr1CK4rzrSgTwV6p/XNv3WlVTq0l4GOVlDqcCsZoTrkWSb 0i0PJ8b1sH0/2tMYJJiB04PP4R7982HsxhqlR1pg87qts238vDrKmwiTVbZva+aV6vFK b5AeIUqEe0y3NoYO07Fq5IcwnmDzXZZbyvn+kkg5+iMlzap5I2Q9Am1sKVnvcaPXL6aZ coOF2wThL9apH3D3qXAheAiNSkXDtQgZiUMxqlzgVQIa6WYfiFaeS51q4CqAdNtCcTt0 qzZw== X-Gm-Message-State: ANhLgQ3UtkDIxoTXuJh/DnXa4hYaEJ1X9V/mI9LQq+wW2yhvAqIut/Vl QJ/x7k2lKLGrRqHdNS21cOJ2VB7I X-Google-Smtp-Source: ADFU+vuApkqG6oJCCvRzScz8TrIt0Cih+bbHg2YjHwih0f3kupyMV9MTWUnbFHNhBLxr/ju1y1/Bfw== X-Received: by 2002:a1c:4987:: with SMTP id w129mr3217864wma.168.1584639033162; Thu, 19 Mar 2020 10:30:33 -0700 (PDT) Received: from arch-x1c3.cbg.collabora.co.uk ([2a00:5f00:102:0:9665:9cff:feee:aa4d]) by smtp.gmail.com with ESMTPSA id k9sm4534601wrd.74.2020.03.19.10.30.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Mar 2020 10:30:32 -0700 (PDT) From: Emil Velikov To: dri-devel@lists.freedesktop.org Subject: [PATCH v2 2/2] drm: error out with EBUSY when device has existing master Date: Thu, 19 Mar 2020 17:29:30 +0000 Message-Id: <20200319172930.230583-2-emil.l.velikov@gmail.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200319172930.230583-1-emil.l.velikov@gmail.com> References: <20200319172930.230583-1-emil.l.velikov@gmail.com> MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: emil.l.velikov@gmail.com Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" From: Emil Velikov As requested by Adam, provide different error message for when the device has an existing master. An audit of the following projects, shows that the errno is used only for printf() purposes. xorg/xserver xorg/drivers/xf86-video-ati xorg/drivers/xf86-video-amdgpu xorg/drivers/xf86-video-intel xorg/drivers/xf86-video-tegra xorg/drivers/xf86-video-freedreno xorg/drivers/xf86-video-nouveau xorg/drivers/xf86-video-vmwgfx qt/kwin/plasma gtk/mutter/gnomeshell efl/enlightment Cc: Adam Jackson Suggested-by: Adam Jackson Signed-off-by: Emil Velikov Reviewed-by: Adam Jackson --- drivers/gpu/drm/drm_auth.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_auth.c b/drivers/gpu/drm/drm_auth.c index 37cac0a221ff..a312fe1be50c 100644 --- a/drivers/gpu/drm/drm_auth.c +++ b/drivers/gpu/drm/drm_auth.c @@ -250,7 +250,7 @@ int drm_setmaster_ioctl(struct drm_device *dev, void *data, goto out_unlock; if (dev->master) { - ret = -EINVAL; + ret = -EBUSY; goto out_unlock; }