Hi Daniel, I am having a look at this now, as have some time. So, to sum up what I think you want. 1. Re-base and apply the patches (so that the known holes are closed in the Nouveau driver). 2. Add DRIVER_KMS_LEGACY_CONTEXT to include/drm/drmP.h 3. Add DRIVER_KMS_LEGACY_CONTEXT to .driver_features in file drivers/gpu/drm/nouveau/nouveau_drm.h. 4. Change all the hw_lock IOCTL functions to have: + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT)) + return -EINVAL; + 5. Add an igt test, that would induce the crash on platforms that are not patched and have DRIVER_KMS_LEGACY_CONTEXT enabled? Is this about right? Thanks, Peter. On Tue, 2015-03-31 at 16:00 +0200, Daniel Vetter wrote: > 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@xxxxxxxxxx> > 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@xxxxxxxx] On Behalf Of Daniel Vetter > > Sent: Tuesday, March 31, 2015 2:26 PM > > To: Antoine, Peter > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Subject: Re: [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@xxxxxxxxx> > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > > 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. > > > _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
