| Message ID | 20181219192247.29880-1-emil.l.velikov@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm: tweak permission handling | expand |
On Wed, Dec 19, 2018 at 07:22:44PM +0000, Emil Velikov wrote: > Hi all, > > This series relaxes some permission handling we have in core. > > The first patch, swaps the DRM_ROOT_ONLY to DRM_MASTER on DROP_MASTER > ioctls. Thus any application can drop privileges just after SET_MASTER > and not worry about elevating them, solely for DROP_MASTER. > > The last commit, admittedly works around userspace bugs. Although it's > far better than the "run as root" approach that people have been using. > > It has the extra side effect of allowing some userspace (but not all) > to use vgem without any modifications ;-) > > Would be great if this series is checked through the Intel GFX trybot > but I'm not sure how to do that. Just cc intel-gfx@lists.freedesktop.org. -Daniel > > Any comments, review or general ack's are appreciated. > > Thanks > Emil > > Emil Velikov (3): > drm: change DROP_MASTER permissions to allow DRM_MASTER > drm: annotate drm_core_check_feature() dev arg. as const > drm: allow render capable master with DRM_AUTH ioctls > > drivers/gpu/drm/drm_ioctl.c | 10 +++++++--- > include/drm/drm_drv.h | 2 +- > 2 files changed, 8 insertions(+), 4 deletions(-) > > -- > 2.19.2 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Dec 19, 2018 at 09:30:46PM +0100, Daniel Vetter wrote: > On Wed, Dec 19, 2018 at 07:22:44PM +0000, Emil Velikov wrote: > > Hi all, > > > > This series relaxes some permission handling we have in core. > > > > The first patch, swaps the DRM_ROOT_ONLY to DRM_MASTER on DROP_MASTER > > ioctls. Thus any application can drop privileges just after SET_MASTER > > and not worry about elevating them, solely for DROP_MASTER. > > > > The last commit, admittedly works around userspace bugs. Although it's > > far better than the "run as root" approach that people have been using. > > > > It has the extra side effect of allowing some userspace (but not all) > > to use vgem without any modifications ;-) > > > > Would be great if this series is checked through the Intel GFX trybot > > but I'm not sure how to do that. > > Just cc intel-gfx@lists.freedesktop.org. Even better would be a few igts to exercise this stuff. We have some basic auth tests, but not much, so running this through the intel CI won't test much at all. -Daniel > -Daniel > > > > > Any comments, review or general ack's are appreciated. > > > > Thanks > > Emil > > > > Emil Velikov (3): > > drm: change DROP_MASTER permissions to allow DRM_MASTER > > drm: annotate drm_core_check_feature() dev arg. as const > > drm: allow render capable master with DRM_AUTH ioctls > > > > drivers/gpu/drm/drm_ioctl.c | 10 +++++++--- > > include/drm/drm_drv.h | 2 +- > > 2 files changed, 8 insertions(+), 4 deletions(-) > > > > -- > > 2.19.2 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
On Wed, 19 Dec 2018 at 20:37, Daniel Vetter <daniel@ffwll.ch> wrote: > > On Wed, Dec 19, 2018 at 09:30:46PM +0100, Daniel Vetter wrote: > > On Wed, Dec 19, 2018 at 07:22:44PM +0000, Emil Velikov wrote: > > > Hi all, > > > > > > This series relaxes some permission handling we have in core. > > > > > > The first patch, swaps the DRM_ROOT_ONLY to DRM_MASTER on DROP_MASTER > > > ioctls. Thus any application can drop privileges just after SET_MASTER > > > and not worry about elevating them, solely for DROP_MASTER. > > > > > > The last commit, admittedly works around userspace bugs. Although it's > > > far better than the "run as root" approach that people have been using. > > > > > > It has the extra side effect of allowing some userspace (but not all) > > > to use vgem without any modifications ;-) > > > > > > Would be great if this series is checked through the Intel GFX trybot > > > but I'm not sure how to do that. > > > > Just cc intel-gfx@lists.freedesktop.org. Thanks will do. > > Even better would be a few igts to exercise this stuff. We have some basic > auth tests, but not much, so running this through the intel CI won't test > much at all. Right, I was thinking about adding something like the following: - open the primary node - /dev/dri/cardX - ensure it's not authenticated - by default the first client (or one run as root) is - issue a trivial ioctl that's annotated as DRM_AUTH - fail if the ioctl returns with -EACCESS Since IGT is usually the first client (or sometimes ran as root), I'm not quite sure how to achieve the second point. Any ideas are greatly appreciated. Thanks Emil
On Thu, Dec 20, 2018 at 12:56:46PM +0000, Emil Velikov wrote: > On Wed, 19 Dec 2018 at 20:37, Daniel Vetter <daniel@ffwll.ch> wrote: > > > > On Wed, Dec 19, 2018 at 09:30:46PM +0100, Daniel Vetter wrote: > > > On Wed, Dec 19, 2018 at 07:22:44PM +0000, Emil Velikov wrote: > > > > Hi all, > > > > > > > > This series relaxes some permission handling we have in core. > > > > > > > > The first patch, swaps the DRM_ROOT_ONLY to DRM_MASTER on DROP_MASTER > > > > ioctls. Thus any application can drop privileges just after SET_MASTER > > > > and not worry about elevating them, solely for DROP_MASTER. > > > > > > > > The last commit, admittedly works around userspace bugs. Although it's > > > > far better than the "run as root" approach that people have been using. > > > > > > > > It has the extra side effect of allowing some userspace (but not all) > > > > to use vgem without any modifications ;-) > > > > > > > > Would be great if this series is checked through the Intel GFX trybot > > > > but I'm not sure how to do that. > > > > > > Just cc intel-gfx@lists.freedesktop.org. > Thanks will do. > > > > > Even better would be a few igts to exercise this stuff. We have some basic > > auth tests, but not much, so running this through the intel CI won't test > > much at all. > > Right, I was thinking about adding something like the following: > - open the primary node - /dev/dri/cardX > - ensure it's not authenticated - by default the first client (or one > run as root) is > - issue a trivial ioctl that's annotated as DRM_AUTH > - fail if the ioctl returns with -EACCESS > > Since IGT is usually the first client (or sometimes ran as root), I'm > not quite sure how to achieve the second point. > Any ideas are greatly appreciated. Open fd a 2nd time, before closing the first one. For examples see the various core_* tests, specically core_auth. -Daniel