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

[chris at chris-wilson.co.uk]

On Tue, Apr 28, 2015 at 10:21:49AM +0100, Dave Gordon wrote:
> On 24/04/15 06:52, Antoine, Peter wrote:
> > I picked up this work due to the following Jira ticket created by the
> > security team (on Android) and was asked to give it a second look and
> > found a few more issues with the hw lock code.
> > 
> > https://jira01.devtools.intel.com/browse/GMINL-5388
> > I/O control on /dev/dri/card0 crashes the kernel (0x4008642b)
> > 
> > It also stops Linux as it kills the driver, I guess it might be possible
> > to reload the gfx driver. On a unpatched system the test that is
> > included in the issue or the igt test that has been posted for the issue
> > will show the problem.
> > 
> > I ran the test on an unpatched system here and the gui stopped and the
> > keyboard stopped responding, so I rebooted. With the patched system I
> > did not need to reboot.
> > 
> > Should I change the SIGTERM to SIGSEGV, not quite the same thing but
> > tooling is better at handling a segfault than a SIGTERM and the
> > application that calls this IOCTL is using an uninitialised hw lock so
> > it is kind of the same as differencing an uninitialised pointer (kind
> > of). Or, I could just remove it, but the bug has been in the code for at
> > least two years (and known about), and I would guess that any code that
> > is calling this is fuzzing the IOCTLs (as this is how the security team
> > found it) and we should reward them with a application exit.
> > 
> > Peter. 
> 
> SIGSEGV would be a better choice.
> 
> SIGTERM is normally sent by a user -- it's the default signal sent by
> kill(1). It's also commonly used to tell a long-running daemon process
> to tidy up and exit cleanly.
> 
> SIGSEGV commonly means "you accessed something that doesn't exist/isn't
> mapped/you don't have permissions for". There are specific subcases that
> can be indicated via the siginfo data; this is from the sigaction(1)
> manpage:
> 
>     The following values can be placed in si_code for a SIGSEGV signal:
> 
>         SEGV_MAPERR    address not mapped to object
> 
>         SEGV_ACCERR    invalid permissions for mapped object
> 
> SIGBUS would also be a possibility but that's generally taken to mean
> that an access got all the way to some physical bus and then faulted,
> whereas SIGSEGV suggests the access was rejected during the
> virtual-to-physical mapping process.

None of the above. Just return -EINVAL, -EPERM, -EACCESS as appropriate.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre