2013/3/17 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>: > On Mon, Mar 18, 2013 at 07:42:58AM +1000, Dave Airlie wrote: >> On Mon, Mar 18, 2013 at 7:40 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >> > On Sun, Mar 17, 2013 at 08:50:03PM +0100, Daniel Vetter wrote: >> >> On Sat, Mar 16, 2013 at 11:19 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >> >> > 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. >> >> >> >> Doesn't that mean that we need these checks everywhere? Or at least a >> >> fixup in drm core proper? >> > >> > That's my conclusion. We either add a flag to ask drm_ioctl to prevent >> > passing NULL pointers (as the existing behaviour may be useful >> > somewhere, and I have not checked all callees) or saturate our callbacks >> > with NULL checks. >> >> Do we have the kernel's expected IOC_IN/IOC_OUT flags at that point as well? >> >> we could check them and block NULL in that case. > > Yes. For the core ioctls, we use drm_ioctls[nr].cmd rather than the > value passed in by userspace for the IOC_IN|IN_OUT bits. So: Thanks, trinity can indeed set the in/out bits randomly, in a way that does not match the driver ioctl definition. Your patch almost fixes this. For the driver ioctls we will want to grab the cmd from cmd_drv. So the patch should be: diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 25f91cd..5210f33 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -408,6 +408,7 @@ long drm_ioctl(struct file *filp, usize = asize = _IOC_SIZE(cmd); if (drv_size > asize) asize = drv_size; + cmd = ioctl->cmd_drv; } else if ((nr >= DRM_COMMAND_END) || (nr < DRM_COMMAND_BASE)) { ioctl = &drm_ioctls[nr]; Can you please submit this officially? > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index 25f91cd..79b8bd1 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -408,6 +408,7 @@ long drm_ioctl(struct file *filp, > usize = asize = _IOC_SIZE(cmd); > if (drv_size > asize) > asize = drv_size; > + cmd = ioctl->cmd; > } > else if ((nr >= DRM_COMMAND_END) || (nr < DRM_COMMAND_BASE)) { > ioctl = &drm_ioctls[nr]; > > > -- > Chris Wilson, Intel Open Source Technology Centre _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel