On Fri, Mar 15, 2013 at 04:49:42PM -0700, Ben Widawsky wrote: > On Fri, Mar 15, 2013 at 10:06:19PM +0000, Chris Wilson wrote: > > On Fri, Mar 15, 2013 at 09:36:07AM -0700, Ben Widawsky wrote: > > > On Fri, Mar 15, 2013 at 08:24:03AM +0000, Chris Wilson wrote: > > > > That's what I thought too. Looking at the stack trace, the empirical > > > > evidence is that we need the check. > > > > -Chris > > > > > > I think we need to investigate the issue more then, or put a BUG_ON() in > > > the drm code and run it through trinity. We have other places where arg > > > can't/shouldn't be NULL and we don't check. > > > > Actually we are both wrong. drm_ioctl() does not validate that the > > user struct matches the expected size, just ensures that if that user > > cmd specifies that the arg is to be used that it only up to the known > > size is copied. > > > > A hostile userspace can bludgen a NULL pointer through drm_ioctl() into > > the driver->ioctl->func(). > > > > > + if (args == NULL) > > > > + return -EINVAL; > > > > + > > I must be failing to see the obvious, but I'm still not getting how args > can ever be NULL. kdata which is passed to us as "data" and cast to > "args' is either always some stack variable, or some kmalloc'd memory. I > see how the arguments themselves can be crap which is really only when > user size != drv_size. > > So tell me, which case can result in a NULL arg? > 1. user size == drv_size // better not be this one > 2. user size < driver size > 3. user size > driver size > > It seems to me we still must [simply] be missing something in our > parameter validation. If *userspace* doesn't request either IOC_IN | IOC_OUT in their ioctl command (which are seperate from the ioctl number), then kdata is set to NULL. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
