linux-kernel - Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130315234942.GB715@bwidawsk.net>
Date:	Fri, 15 Mar 2013 16:49:42 -0700
From:	Ben Widawsky <ben@...dawsk.net>
To:	Chris Wilson <chris@...is-wilson.co.uk>,
	Tommi Rantala <tt.rantala@...il.com>,
	David Airlie <airlied@...ux.ie>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	dri-devel@...ts.freedesktop.org, Dave Jones <davej@...hat.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data
 for a NULL pointer

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 we used driver->ioctl->cmd instead of the user supplied cmd, we would
> have a whole other can of worms to deal with (as we suddenly get a
> struct of zeroes). However, those worms should already be treated as
> invalid. Choose your poison.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre


-- 
Ben Widawsky, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/