On Wed, Jul 10, 2013 at 6:27 PM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
