Re: [PATCH v2 1/2] drm: Make HW_LOCK access functions optional.

Daniel Vetter
2015-05-13T07:24:21Z

Adding dri-devel, I've forgotten to do that ...
-Daniel

On Wed, May 13, 2015 at 09:14:29AM +0200, Daniel Vetter wrote:
> On Wed, May 13, 2015 at 07:54:47AM +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.
>
> Is there a reference for why nouveau needs hw locks too? Also have you
> done an audit of mesa history and X history to make sure there's no other
> driver accidentally using it with a modern kms driver?
>
> > Issue: VIZ-5485
> > Signed-off-by: Peter Antoine <peter.antoine@intel.com>
> > ---
> > drivers/gpu/drm/drm_lock.c | 6 ++++++
> > drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++-
> > include/drm/drmP.h | 23 ++++++++++++-----------
> > 3 files changed, 20 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c
> > index f861361..21eb180 100644
> > --- a/drivers/gpu/drm/drm_lock.c
> > +++ b/drivers/gpu/drm/drm_lock.c
> > @@ -61,6 +61,9 @@ int drm_legacy_lock(struct drm_device *dev, void *data,
> > struct drm_master *master = file_priv->master;
> > int ret = 0;
> >
> > + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
>
> You also need to allow these for all legacy drivers, i.e. without
> DRIVER_MODESET.
> -Daniel
>
> > + return -EINVAL;
> > +
> > ++file_priv->lock_count;
> >
> > if (lock->context == DRM_KERNEL_CONTEXT) {
> > @@ -153,6 +156,9 @@ int drm_legacy_unlock(struct drm_device *dev, void *data, struct drm_file *file_
> > struct drm_lock *lock = data;
> > struct drm_master *master = file_priv->master;
> >
> > + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT))
> > + return -EINVAL;
> > +
> > if (lock->context == DRM_KERNEL_CONTEXT) {
> > DRM_ERROR("Process %d using kernel context %d\n",
> > task_pid_nr(current), lock->context);
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > index 8904933..9624b38 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -941,7 +941,8 @@ static struct drm_driver
> > driver_stub = {
> > .driver_features =
> > DRIVER_USE_AGP |
> > - DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER,
> > + DRIVER_GEM | DRIVER_MODESET | DRIVER_PRIME | DRIVER_RENDER |
> > + DRIVER_KMS_LEGACY_CONTEXT,
> >
> > .load = nouveau_drm_load,
> > .unload = nouveau_drm_unload,
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index df6d997..3874942 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -137,17 +137,18 @@ void drm_err(const char *format, ...);
> > /*@{*/
> >
> > /* driver capabilities and requirements mask */
> > -#define DRIVER_USE_AGP 0x1
> > -#define DRIVER_PCI_DMA 0x8
> > -#define DRIVER_SG 0x10
> > -#define DRIVER_HAVE_DMA 0x20
> > -#define DRIVER_HAVE_IRQ 0x40
> > -#define DRIVER_IRQ_SHARED 0x80
> > -#define DRIVER_GEM 0x1000
> > -#define DRIVER_MODESET 0x2000
> > -#define DRIVER_PRIME 0x4000
> > -#define DRIVER_RENDER 0x8000
> > -#define DRIVER_ATOMIC 0x10000
> > +#define DRIVER_USE_AGP 0x1
> > +#define DRIVER_PCI_DMA 0x8
> > +#define DRIVER_SG 0x10
> > +#define DRIVER_HAVE_DMA 0x20
> > +#define DRIVER_HAVE_IRQ 0x40
> > +#define DRIVER_IRQ_SHARED 0x80
> > +#define DRIVER_GEM 0x1000
> > +#define DRIVER_MODESET 0x2000
> > +#define DRIVER_PRIME 0x4000
> > +#define DRIVER_RENDER 0x8000
> > +#define DRIVER_ATOMIC 0x10000
> > +#define DRIVER_KMS_LEGACY_CONTEXT 0x20000
> >
> > /***********************************************************************/
> > /** \name Macros to make printk easier */
> > --
> > 1.9.1
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org