lkml.org 
[lkml]   [2013]   [Mar]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer
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@chris-wilson.co.uk> 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@chris-wilson.co.uk> 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:

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


\
 
 \ /
  Last update: 2013-03-17 23:21    [W:0.052 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site