From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: bug report: potential integer overflow in validate_exec_list() Date: Mon, 22 Nov 2010 10:35:10 +0000 Message-ID: <849307$afcbkr@azsmga001.ch.intel.com> References: <20101120183207.GC1522@bicker> <849307$af353c@azsmga001.ch.intel.com> <20101122095642.GD1522@bicker> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20101122095642.GD1522@bicker> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: Dan Carpenter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, 22 Nov 2010 12:56:42 +0300, Dan Carpenter wrote: > On Sun, Nov 21, 2010 at 09:23:46AM +0000, Chris Wilson wrote: > > Yes, it could. Not through normal use since relocation count can not be > > more than buffer length, hence realistically capped at around 4k entries. > > However... > > > > If the user deliberately made it wrap to get past the access_ok() check > then it would just return -ENOENT in i915_gem_execbuffer_relocate() > right? > > It doesn't look like there are any security implications but I just > wanted to be sure. I think it did have a security implication, because it would only validate the first x bytes of the user pointer but then continue to read/write beyond. It would have to be a fairly crafty user! I've queued a fix in the drm-intel-fixes branch: commit d1d788302e8c76e5138dfa61f4a5eee4f72a748f Author: Chris Wilson Date: Sun Nov 21 09:23:48 2010 +0000 drm/i915: Prevent integer overflow when validating the execbuffer Commit 2549d6c2 removed the vmalloc used for temporary storage of the relocation lists used during execbuffer. However, our use of vmalloc was being protected by an integer overflow check which we do want to preserve! Reported-by: Dan Carpenter Signed-off-by: Chris Wilson Many thanks, -Chris -- Chris Wilson, Intel Open Source Technology Centre