[PATCH 33/39] drm: rip out drm_core_has_MTRR checks

[Daniel Vetter]

On Wed, Jul 10, 2013 at 6:27 PM, Andy Lutomirski <luto at amacapital.net> wrote:
> Are all of those codepaths really inaccessible in non-legacy drm
> drivers?  I didn't try to fully unravel all the ioctls and such, but
> it seems like userspace could add bufs and map them.  Since the mtrr
> code isn't very robust (reference counting?  what reference
> counting?), I'm a little bit worried that potentially enabling it in
> more cases, which your patch does, could be harmful.
>
> The arch_phys_wc stuff puts a prettier interface on the mtrr code and
> turns it off when PAT is available, but the underlying code is still
> just as bad.

Well, the entire drm bufs stuff isn't refcounted and there are indeed
legacy driver that abused this in a completely unsafe way. E.g. for
i810.ko the ddx driver in userspace creates a register mapping through
the addbuf ioctl, which the kernel driver then uses. With no
refcounting at all to prevent an Oops (and I've seen them happen, you
simply need to kill X).

So I don't think this patch will make matters worse, especially since
most drivers set DRIVER_USE_MTRR. The way to fix this up is to
holesale block out these unsafe ioctls for kernel modesetting driver,
which this series here does for a lot of cases (still a bunch of them
left though). There's no way we can fix up the ums drm drivers without
breaking userspace :(

I haven't yet gotten around to blocking out the addmap ioctls since
reviewing existing userspace code will be a real pain. But at least
addmap is restrict to CAP_SYS_ADMIN, so not a that grave exploit
issue. But I very much plan to do that audit and then disable addmap
and friends for kms drivers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch