diff mbox series

[FFmpeg-devel] kmsgrab: Drop DRM master if running as root

Message ID 4e96dd48-088b-f9a5-f2e7-6d1b4bf9c42c@jkqxz.net
State New
Headers show
Series [FFmpeg-devel] kmsgrab: Drop DRM master if running as root | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Mark Thompson April 12, 2020, 2:46 p.m. UTC 
If we have both root and DRM master then drop the latter because we
don't need both and holding onto it can interfere with running other
programs.

From a discussion with deltasquared on IRC.
---
It would be marginally better to check whether we have CAP_SYS_ADMIN rather than just whether we are effectively root, but the capabilities interface is a lot more complex to query and this is already sufficient to cover the reported case.

 libavdevice/kmsgrab.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Anton Khirnov April 14, 2020, 9:02 a.m. UTC | #1 
Quoting Mark Thompson (2020-04-12 16:46:25)
> If we have both root and DRM master then drop the latter because we
> don't need both and holding onto it can interfere with running other
> programs.
> 
> From a discussion with deltasquared on IRC.
> ---
> It would be marginally better to check whether we have CAP_SYS_ADMIN rather than just whether we are effectively root, but the capabilities interface is a lot more complex to query and this is already sufficient to cover the reported case.
> 
>  libavdevice/kmsgrab.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
> index d0de774871..53280da0f5 100644
> --- a/libavdevice/kmsgrab.c
> +++ b/libavdevice/kmsgrab.c
> @@ -263,6 +263,13 @@ static av_cold int kmsgrab_read_header(AVFormatContext *avctx)
>      ctx->device = (AVHWDeviceContext*) ctx->device_ref->data;
>      ctx->hwctx  = (AVDRMDeviceContext*)ctx->device->hwctx;
>  
> +    if (geteuid() == 0 && drmIsMaster(ctx->hwctx->fd)) {
> +        // If we have both root and DRM master then drop the latter
> +        // because we don't need both and holding onto it can interfere
> +        // with running other programs.
> +        drmDropMaster(ctx->hwctx->fd);
> +    }

Any idea how would this interact with user namespaces?
diff mbox series

Patch

diff --git a/libavdevice/kmsgrab.c b/libavdevice/kmsgrab.c
index d0de774871..53280da0f5 100644
--- a/libavdevice/kmsgrab.c
+++ b/libavdevice/kmsgrab.c
@@ -263,6 +263,13 @@  static av_cold int kmsgrab_read_header(AVFormatContext *avctx)
     ctx->device = (AVHWDeviceContext*) ctx->device_ref->data;
     ctx->hwctx  = (AVDRMDeviceContext*)ctx->device->hwctx;
 
+    if (geteuid() == 0 && drmIsMaster(ctx->hwctx->fd)) {
+        // If we have both root and DRM master then drop the latter
+        // because we don't need both and holding onto it can interfere
+        // with running other programs.
+        drmDropMaster(ctx->hwctx->fd);
+    }
+
     err = drmSetClientCap(ctx->hwctx->fd,
                           DRM_CLIENT_CAP_UNIVERSAL_PLANES, 1);
     if (err < 0) {