Re: [PATCH 09/11] drm: replace drawable ioctl by noops

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 29, 2010 at 5:14 AM, Daniel Vetter <daniel.vetter@xxxxxxxx> wrote:
> The information supplied by userspace through these ioctls is only
> accessible by dev->drw_idr. But there's no in-tree user of that.
> Information might also leak via the RM_DRAW ioctl (in the form of
> a negative return code in case the drawable doesn't exist). But the
> only user of these ioctls, mesas' miniglx, does not use the RM_DRAW
> ioctl.
>
> Therefore it's safe to replace these three ioctls with noops and rip
> out the implementation.

Looks almost exactly like this one:

http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg39176.html

Except I made adddraw a stub that always returns 1, but reading
through the code (hw/xfree86/dri, glx/glxdri.c and the radeon dri
driver) I don't see anything that fails if we just return zero (or a
random number for that matter).  The only difference is that if we
return zero, hw/xfree86/dri/dri.c will keep trying to create a drm
drawable (look for drmCreateDrawable) every time somebody creates a
dri drawable, but since that's a noop, who cares.

Reviewed-by: Kristian Høgsberg <krh@xxxxxxxxxxxxx>

> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
>  drivers/gpu/drm/Makefile       |    2 +-
>  drivers/gpu/drm/drm_drawable.c |  197 ----------------------------------------
>  drivers/gpu/drm/drm_drv.c      |    8 +-
>  drivers/gpu/drm/drm_stub.c     |    3 -
>  include/drm/drmP.h             |   15 ---
>  5 files changed, 4 insertions(+), 221 deletions(-)
>  delete mode 100644 drivers/gpu/drm/drm_drawable.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index abe3f44..10585ce 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -5,7 +5,7 @@
>  ccflags-y := -Iinclude/drm
>
>  drm-y       := drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
> -               drm_context.o drm_dma.o drm_drawable.o \
> +               drm_context.o drm_dma.o \
>                drm_drv.o drm_fops.o drm_gem.o drm_ioctl.o drm_irq.o \
>                drm_lock.o drm_memory.o drm_proc.o drm_stub.o drm_vm.o \
>                drm_agpsupport.o drm_scatter.o ati_pcigart.o drm_pci.o \
> diff --git a/drivers/gpu/drm/drm_drawable.c b/drivers/gpu/drm/drm_drawable.c
> deleted file mode 100644
> index 170e531..0000000
> --- a/drivers/gpu/drm/drm_drawable.c
> +++ /dev/null
> @@ -1,197 +0,0 @@
> -/**
> - * \file drm_drawable.c
> - * IOCTLs for drawables
> - *
> - * \author Rickard E. (Rik) Faith <faith@xxxxxxxxxxx>
> - * \author Gareth Hughes <gareth@xxxxxxxxxxx>
> - * \author Michel Dänzer <michel@xxxxxxxxxxxxxxxxxxxx>
> - */
> -
> -/*
> - * Created: Tue Feb  2 08:37:54 1999 by faith@xxxxxxxxxxx
> - *
> - * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> - * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> - * Copyright 2006 Tungsten Graphics, Inc., Bismarck, North Dakota.
> - * All Rights Reserved.
> - *
> - * Permission is hereby granted, free of charge, to any person obtaining a
> - * copy of this software and associated documentation files (the "Software"),
> - * to deal in the Software without restriction, including without limitation
> - * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> - * and/or sell copies of the Software, and to permit persons to whom the
> - * Software is furnished to do so, subject to the following conditions:
> - *
> - * The above copyright notice and this permission notice (including the next
> - * paragraph) shall be included in all copies or substantial portions of the
> - * Software.
> - *
> - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> - * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> - * OTHER DEALINGS IN THE SOFTWARE.
> - */
> -
> -#include "drmP.h"
> -
> -/**
> - * Allocate drawable ID and memory to store information about it.
> - */
> -int drm_adddraw(struct drm_device *dev, void *data, struct drm_file *file_priv)
> -{
> -       unsigned long irqflags;
> -       struct drm_draw *draw = data;
> -       int new_id = 0;
> -       int ret;
> -
> -again:
> -       if (idr_pre_get(&dev->drw_idr, GFP_KERNEL) == 0) {
> -               DRM_ERROR("Out of memory expanding drawable idr\n");
> -               return -ENOMEM;
> -       }
> -
> -       spin_lock_irqsave(&dev->drw_lock, irqflags);
> -       ret = idr_get_new_above(&dev->drw_idr, NULL, 1, &new_id);
> -       if (ret == -EAGAIN) {
> -               spin_unlock_irqrestore(&dev->drw_lock, irqflags);
> -               goto again;
> -       }
> -
> -       spin_unlock_irqrestore(&dev->drw_lock, irqflags);
> -
> -       draw->handle = new_id;
> -
> -       DRM_DEBUG("%d\n", draw->handle);
> -
> -       return 0;
> -}
> -
> -/**
> - * Free drawable ID and memory to store information about it.
> - */
> -int drm_rmdraw(struct drm_device *dev, void *data, struct drm_file *file_priv)
> -{
> -       struct drm_draw *draw = data;
> -       unsigned long irqflags;
> -       struct drm_drawable_info *info;
> -
> -       spin_lock_irqsave(&dev->drw_lock, irqflags);
> -
> -       info = drm_get_drawable_info(dev, draw->handle);
> -       if (info == NULL) {
> -               spin_unlock_irqrestore(&dev->drw_lock, irqflags);
> -               return -EINVAL;
> -       }
> -       kfree(info->rects);
> -       kfree(info);
> -
> -       idr_remove(&dev->drw_idr, draw->handle);
> -
> -       spin_unlock_irqrestore(&dev->drw_lock, irqflags);
> -       DRM_DEBUG("%d\n", draw->handle);
> -       return 0;
> -}
> -
> -int drm_update_drawable_info(struct drm_device *dev, void *data, struct drm_file *file_priv)
> -{
> -       struct drm_update_draw *update = data;
> -       unsigned long irqflags;
> -       struct drm_clip_rect *rects;
> -       struct drm_drawable_info *info;
> -       int err;
> -
> -       info = idr_find(&dev->drw_idr, update->handle);
> -       if (!info) {
> -               info = kzalloc(sizeof(*info), GFP_KERNEL);
> -               if (!info)
> -                       return -ENOMEM;
> -               if (IS_ERR(idr_replace(&dev->drw_idr, info, update->handle))) {
> -                       DRM_ERROR("No such drawable %d\n", update->handle);
> -                       kfree(info);
> -                       return -EINVAL;
> -               }
> -       }
> -
> -       switch (update->type) {
> -       case DRM_DRAWABLE_CLIPRECTS:
> -               if (update->num == 0)
> -                       rects = NULL;
> -               else if (update->num != info->num_rects) {
> -                       rects = kmalloc(update->num *
> -                                       sizeof(struct drm_clip_rect),
> -                                       GFP_KERNEL);
> -               } else
> -                       rects = info->rects;
> -
> -               if (update->num && !rects) {
> -                       DRM_ERROR("Failed to allocate cliprect memory\n");
> -                       err = -ENOMEM;
> -                       goto error;
> -               }
> -
> -               if (update->num && DRM_COPY_FROM_USER(rects,
> -                                                    (struct drm_clip_rect __user *)
> -                                                    (unsigned long)update->data,
> -                                                    update->num *
> -                                                    sizeof(*rects))) {
> -                       DRM_ERROR("Failed to copy cliprects from userspace\n");
> -                       err = -EFAULT;
> -                       goto error;
> -               }
> -
> -               spin_lock_irqsave(&dev->drw_lock, irqflags);
> -
> -               if (rects != info->rects) {
> -                       kfree(info->rects);
> -               }
> -
> -               info->rects = rects;
> -               info->num_rects = update->num;
> -
> -               spin_unlock_irqrestore(&dev->drw_lock, irqflags);
> -
> -               DRM_DEBUG("Updated %d cliprects for drawable %d\n",
> -                         info->num_rects, update->handle);
> -               break;
> -       default:
> -               DRM_ERROR("Invalid update type %d\n", update->type);
> -               return -EINVAL;
> -       }
> -
> -       return 0;
> -
> -error:
> -       if (rects != info->rects)
> -               kfree(rects);
> -
> -       return err;
> -}
> -
> -/**
> - * Caller must hold the drawable spinlock!
> - */
> -static struct drm_drawable_info *drm_get_drawable_info(struct drm_device *dev, drm_drawable_t id)
> -{
> -       return idr_find(&dev->drw_idr, id);
> -}
> -
> -static int drm_drawable_free(int idr, void *p, void *data)
> -{
> -       struct drm_drawable_info *info = p;
> -
> -       if (info) {
> -               kfree(info->rects);
> -               kfree(info);
> -       }
> -
> -       return 0;
> -}
> -
> -void drm_drawable_free_all(struct drm_device *dev)
> -{
> -       idr_for_each(&dev->drw_idr, drm_drawable_free, NULL);
> -       idr_remove_all(&dev->drw_idr);
> -}
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 4a66201..ea4b6c2 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -88,8 +88,8 @@ static struct drm_ioctl_desc drm_ioctls[] = {
>        DRM_IOCTL_DEF(DRM_IOCTL_NEW_CTX, drm_newctx, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>        DRM_IOCTL_DEF(DRM_IOCTL_RES_CTX, drm_resctx, DRM_AUTH),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_ADD_DRAW, drm_adddraw, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> -       DRM_IOCTL_DEF(DRM_IOCTL_RM_DRAW, drm_rmdraw, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +       DRM_IOCTL_DEF(DRM_IOCTL_ADD_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +       DRM_IOCTL_DEF(DRM_IOCTL_RM_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
>        DRM_IOCTL_DEF(DRM_IOCTL_LOCK, drm_lock, DRM_AUTH),
>        DRM_IOCTL_DEF(DRM_IOCTL_UNLOCK, drm_unlock, DRM_AUTH),
> @@ -124,7 +124,7 @@ static struct drm_ioctl_desc drm_ioctls[] = {
>
>        DRM_IOCTL_DEF(DRM_IOCTL_MODESET_CTL, drm_modeset_ctl, 0),
>
> -       DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_update_drawable_info, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
> +       DRM_IOCTL_DEF(DRM_IOCTL_UPDATE_DRAW, drm_noop, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
>
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_CLOSE, drm_gem_close_ioctl, DRM_UNLOCKED),
>        DRM_IOCTL_DEF(DRM_IOCTL_GEM_FLINK, drm_gem_flink_ioctl, DRM_AUTH|DRM_UNLOCKED),
> @@ -177,8 +177,6 @@ int drm_lastclose(struct drm_device * dev)
>
>        mutex_lock(&dev->struct_mutex);
>
> -       /* Free drawable information memory */
> -       drm_drawable_free_all(dev);
>        del_timer(&dev->timer);
>
>        /* Clear AGP information */
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index b743411..e3b5cfc 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -237,14 +237,11 @@ static int drm_fill_in_dev(struct drm_device * dev, struct pci_dev *pdev,
>        INIT_LIST_HEAD(&dev->vblank_event_list);
>
>        spin_lock_init(&dev->count_lock);
> -       spin_lock_init(&dev->drw_lock);
>        spin_lock_init(&dev->event_lock);
>        init_timer(&dev->timer);
>        mutex_init(&dev->struct_mutex);
>        mutex_init(&dev->ctxlist_mutex);
>
> -       idr_init(&dev->drw_idr);
> -
>        dev->pdev = pdev;
>        dev->pci_device = pdev->device;
>        dev->pci_vendor = pdev->vendor;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index af62612..540cab6 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1026,12 +1026,6 @@ struct drm_device {
>        struct drm_minor *control;              /**< Control node for card */
>        struct drm_minor *primary;              /**< render type primary screen head */
>
> -       /** \name Drawable information */
> -       /*@{ */
> -       spinlock_t drw_lock;
> -       struct idr drw_idr;
> -       /*@} */
> -
>         struct drm_mode_config mode_config;    /**< Current mode config */
>
>        /** \name GEM information */
> @@ -1202,15 +1196,6 @@ extern int drm_setsareactx(struct drm_device *dev, void *data,
>  extern int drm_getsareactx(struct drm_device *dev, void *data,
>                           struct drm_file *file_priv);
>
> -                               /* Drawable IOCTL support (drm_drawable.h) */
> -extern int drm_adddraw(struct drm_device *dev, void *data,
> -                      struct drm_file *file_priv);
> -extern int drm_rmdraw(struct drm_device *dev, void *data,
> -                     struct drm_file *file_priv);
> -extern int drm_update_drawable_info(struct drm_device *dev, void *data,
> -                                   struct drm_file *file_priv);
> -extern void drm_drawable_free_all(struct drm_device *dev);
> -
>                                /* Authentication IOCTL support (drm_auth.h) */
>  extern int drm_getmagic(struct drm_device *dev, void *data,
>                        struct drm_file *file_priv);
> --
> 1.7.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux