[Intel-gfx] [PATCH] drm: Kernel Crash in drm_unlock

[Antoine, Peter]

Probably. I'll need to check this end.
I'll have a look.

Peter.

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of Daniel Vetter
Sent: Tuesday, March 31, 2015 3:01 PM
To: Antoine, Peter
Cc: Daniel Vetter; intel-gfx at lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm: Kernel Crash in drm_unlock

On Tue, Mar 31, 2015 at 01:34:25PM +0000, Antoine, Peter wrote:
> This was found by the security guys using an ioctl fuzzer.
> 12 lines of code from a new unprivileged user and the kernel goes bang.
>   
> The other crash was just found using code inspection, but it is the same basic issue.
> Either the hw_lock was not created or the was deleted and the pointer is dereferenced.
> 
> For the escalation, there is not proof of concept, but it is a bad 
> comparison as the bits are stripped off for other checks.
> 
> I'll be re-spinning the patches when I get notified that I am on the 
> no footer list.

In that case I think an igt testcase to make this go boom would be great.
Testbinary prefix for drm core is drm_ (there's some already).

Meanwhile I did dig out the history for this and it's not pretty. See

commit c21eb21cb50d58e7cbdcb8b9e7ff68b85cfa5095
Author: Dave Airlie <airlied at redhat.com>
Date:   Fri Sep 20 08:32:59 2013 +1000

    Revert "drm: mark context support as a legacy subsystem"

Imo the correct way to fix this isn't to try to fix the code (it's hopeless, making it go boom with fuzzing is just the tip of the iceberg), but instead to disable it. But we may not break nouvea, so needs a bit more elaborate:
1. Add DRIVER_KMS_LEGACY_CONTEXT driver flag and add it to nouveau.
2. Modify all the DRIVER_MODESET checks from my patch
(7c510133d93dd6f15ca040733ba7b2891ed61fd1) to still let the ioctls through when DRIVER_KMS_LEGACY_CONTEXT is set.

Can you please sign up for this plus the minimal igt?

Thanks, Daniel
> 
> Peter.
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter at ffwll.ch] On Behalf Of 
> Daniel Vetter
> Sent: Tuesday, March 31, 2015 2:26 PM
> To: Antoine, Peter
> Cc: intel-gfx at lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm: Kernel Crash in drm_unlock
> 
> On Tue, Mar 31, 2015 at 09:09:33AM +0100, Peter Antoine wrote:
> > This patch fixes a possible kernel crash when drm_unlock
> > (DRM_IOCTL_UNLOCK) is called by a application that has not had a 
> > lock created by it. This crash can be caused by any application from all users.
> > 
> > Issue: GMINL-7446
> > Change-Id: I901ff713be53c5ec1c9eaf7ee0ff4314a659af05
> > Signed-off-by: Peter Antoine <peter.antoine at intel.com>
> 
> Can you really blow this up at runtime with modern modeset drivers like i915? Counts for all three patches ...
> 
> > ---
> >  drivers/gpu/drm/drm_lock.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_lock.c b/drivers/gpu/drm/drm_lock.c 
> > index f645268..80253a7 100644
> > --- a/drivers/gpu/drm/drm_lock.c
> > +++ b/drivers/gpu/drm/drm_lock.c
> > @@ -156,6 +156,14 @@ int drm_unlock(struct drm_device *dev, void 
> > *data, struct drm_file *file_priv)
> 
> Also please rebase to latest upstream when submitting patches to the public (the function is now called drm_legacy_unlock).
> 
> >  		return -EINVAL;
> >  	}
> >  
> > +	if (!master->lock.hw_lock) {
> > +		DRM_ERROR(
> > +			"Device has been unregistered. Hard exit. Process %d\n",
> > +			task_pid_nr(current));
> > +		send_sig(SIGTERM, current, 0);
> > +		return -EINTR;
> > +	}
> > +
> >  	if (drm_lock_free(&master->lock, lock->context)) {
> >  		/* FIXME: Should really bail out here. */
> >  	}
> > --
> > 1.9.1
> > 
> > --------------------------------------------------------------------
> > -
> > Intel Corporation (UK) Limited
> > Registered No. 1134945 (England)
> > Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
> > 
> > This e-mail and any attachments may contain confidential material 
> > for the sole use of the intended recipient(s). Any review or 
> > distribution by others is strictly prohibited. If you are not the 
> > intended recipient, please contact the sender and delete all copies.
> 
> And please remove this disclaimer.
> 
> Thanks, Daniel
> 
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ---------------------------------------------------------------------
> Intel Corporation (UK) Limited
> Registered No. 1134945 (England)
> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
> 
> This e-mail and any attachments may contain confidential material for 
> the sole use of the intended recipient(s). Any review or distribution 
> by others is strictly prohibited. If you are not the intended 
> recipient, please contact the sender and delete all copies.
> 

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch