[PATCH v2 2/2] drm: Make Legacy Context access functions optional.
Daniel Vetter
daniel at ffwll.ch
Wed May 13 00:19:09 PDT 2015
On Wed, May 13, 2015 at 07:54:48AM +0100, Peter Antoine wrote:
> As these functions are only used by one driver and there are security holes
> in these functions. Make the functions optional.
>
> These changes are based on the two patches:
> commit c21eb21cb50d58e7cbdcb8b9e7ff68b85cfa5095
> Author: Dave Airlie <airlied at redhat.com>
>
> And the commit that the above patch reverts:
> commit 7c510133d93dd6f15ca040733ba7b2891ed61fd1
> Author: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> This should now turn off the context feature.
>
> Issue: VIZ-5485
> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
Please cc drm core patches to dri-devel. Review below.
-Daniel
> ---
> drivers/gpu/drm/drm_context.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/drm_drv.c | 12 +++++++-----
> 2 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> index 9b23525..574be2a 100644
> --- a/drivers/gpu/drm/drm_context.c
> +++ b/drivers/gpu/drm/drm_context.c
> @@ -53,6 +53,9 @@ struct drm_ctx_list {
> */
> void drm_legacy_ctxbitmap_free(struct drm_device * dev, int ctx_handle)
> {
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
This doesn't compile too well. Also like with the previous patch, drivers
without DRIVER_MODESET still need these ioctls.
> +
> mutex_lock(&dev->struct_mutex);
> idr_remove(&dev->ctx_idr, ctx_handle);
> mutex_unlock(&dev->struct_mutex);
> @@ -87,6 +90,9 @@ static int drm_legacy_ctxbitmap_next(struct drm_device * dev)
> */
> int drm_legacy_ctxbitmap_init(struct drm_device * dev)
> {
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
This is redundant with the check you've added around the caller. Looking
at other places wrapping callers of _legacy_ functions with these checks
is the more common pattern in drm.
> +
> idr_init(&dev->ctx_idr);
> return 0;
> }
> @@ -101,6 +107,9 @@ int drm_legacy_ctxbitmap_init(struct drm_device * dev)
> */
> void drm_legacy_ctxbitmap_cleanup(struct drm_device * dev)
> {
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
Same issue with compiling. And probably better to move the check out into
callers.
> +
> mutex_lock(&dev->struct_mutex);
> idr_destroy(&dev->ctx_idr);
> mutex_unlock(&dev->struct_mutex);
> @@ -119,6 +128,9 @@ void drm_legacy_ctxbitmap_flush(struct drm_device *dev, struct drm_file *file)
> {
> struct drm_ctx_list *pos, *tmp;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
Same as above.
> +
> mutex_lock(&dev->ctxlist_mutex);
>
> list_for_each_entry_safe(pos, tmp, &dev->ctxlist, head) {
> @@ -161,6 +173,9 @@ int drm_legacy_getsareactx(struct drm_device *dev, void *data,
> struct drm_local_map *map;
> struct drm_map_list *_entry;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> mutex_lock(&dev->struct_mutex);
>
> map = idr_find(&dev->ctx_idr, request->ctx_id);
> @@ -205,6 +220,9 @@ int drm_legacy_setsareactx(struct drm_device *dev, void *data,
> struct drm_local_map *map = NULL;
> struct drm_map_list *r_list = NULL;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> mutex_lock(&dev->struct_mutex);
> list_for_each_entry(r_list, &dev->maplist, head) {
> if (r_list->map
> @@ -305,6 +323,9 @@ int drm_legacy_resctx(struct drm_device *dev, void *data,
> struct drm_ctx ctx;
> int i;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> if (res->count >= DRM_RESERVED_CONTEXTS) {
> memset(&ctx, 0, sizeof(ctx));
> for (i = 0; i < DRM_RESERVED_CONTEXTS; i++) {
> @@ -335,6 +356,9 @@ int drm_legacy_addctx(struct drm_device *dev, void *data,
> struct drm_ctx_list *ctx_entry;
> struct drm_ctx *ctx = data;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> ctx->handle = drm_legacy_ctxbitmap_next(dev);
> if (ctx->handle == DRM_KERNEL_CONTEXT) {
> /* Skip kernel's context and get a new one. */
> @@ -378,6 +402,9 @@ int drm_legacy_getctx(struct drm_device *dev, void *data,
> {
> struct drm_ctx *ctx = data;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> /* This is 0, because we don't handle any context flags */
> ctx->flags = 0;
>
> @@ -400,6 +427,9 @@ int drm_legacy_switchctx(struct drm_device *dev, void *data,
> {
> struct drm_ctx *ctx = data;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> DRM_DEBUG("%d\n", ctx->handle);
> return drm_context_switch(dev, dev->last_context, ctx->handle);
> }
> @@ -420,6 +450,9 @@ int drm_legacy_newctx(struct drm_device *dev, void *data,
> {
> struct drm_ctx *ctx = data;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> DRM_DEBUG("%d\n", ctx->handle);
> drm_context_switch_complete(dev, file_priv, ctx->handle);
>
> @@ -442,6 +475,9 @@ int drm_legacy_rmctx(struct drm_device *dev, void *data,
> {
> struct drm_ctx *ctx = data;
>
> + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> + return -EINVAL;
> +
> DRM_DEBUG("%d\n", ctx->handle);
> if (ctx->handle != DRM_KERNEL_CONTEXT) {
> if (dev->driver->context_dtor)
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 3b3c4f5..1de2df8 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -584,11 +584,13 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> if (drm_ht_create(&dev->map_hash, 12))
> goto err_minors;
>
> - ret = drm_legacy_ctxbitmap_init(dev);
> - if (ret) {
> - DRM_ERROR("Cannot allocate memory for context bitmap.\n");
> - goto err_ht;
> - }
> + if (drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
Coding style requires {} since it's multi-line. And confusing since the if
(ret) gets always executed.
> + ret = drm_legacy_ctxbitmap_init(dev);
> + if (ret) {
> + DRM_ERROR(
> + "Cannot allocate memory for context bitmap.\n");
> + goto err_ht;
> + }
>
> if (drm_core_check_feature(dev, DRIVER_GEM)) {
> ret = drm_gem_init(dev);
> --
> 1.9.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list