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

[Daniel Vetter]

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 at 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