On 28/04/15 10:21, 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.
[snip]
Nope, I've changed my mind about this. I thought the problematic case was just a process releasing the lock without having acquired it, but on further examination it's really that the DRM master process has gone away or otherwise deleted (or never created?) the lock. And THEN the (non-master?) process tries to release the (now) non-existent lock.
But more importantly, there's existing code in the acquire-lock path that sends SIGTERM for this case, so obviously the release-lock code should be as similar as possible.
.Dave.