From patchwork Sun Mar 17 21:51:42 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 2284441 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork1.kernel.org (Postfix) with ESMTP id 4661540AF6 for ; Sun, 17 Mar 2013 21:52:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2470CE5FF1 for ; Sun, 17 Mar 2013 14:52:05 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 4A76DE5D01; Sun, 17 Mar 2013 14:51:50 -0700 (PDT) Received: from azsmga002.ch.intel.com ([10.2.17.35]) by azsmga102.ch.intel.com with ESMTP; 17 Mar 2013 14:51:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.84,860,1355126400"; d="scan'208";a="215766572" Received: from unknown (HELO cantiga.alporthouse.com) ([10.255.13.138]) by AZSMGA002.ch.intel.com with SMTP; 17 Mar 2013 14:51:46 -0700 Received: by cantiga.alporthouse.com (sSMTP sendmail emulation); Sun, 17 Mar 2013 21:51:42 +0000 Date: Sun, 17 Mar 2013 21:51:42 +0000 From: Chris Wilson To: Dave Airlie Subject: Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer Message-ID: <20130317215142.GC26102@cantiga.alporthouse.com> Mail-Followup-To: Chris Wilson , Dave Airlie , Daniel Vetter , Ben Widawsky , Tommi Rantala , David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Dave Jones References: <1363265997-29023-1-git-send-email-chris@chris-wilson.co.uk> <20130315045004.GA14747@bwidawsk.net> <20130315082403.GA29916@cantiga.alporthouse.com> <20130315163606.GA17773@bwidawsk.net> <20130315220619.GA17405@cantiga.alporthouse.com> <20130315234942.GB715@bwidawsk.net> <20130316101920.GB17405@cantiga.alporthouse.com> <20130317214049.GB26102@cantiga.alporthouse.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: Ben Widawsky , Daniel Vetter , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Dave Jones X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org Errors-To: dri-devel-bounces+patchwork-dri-devel=patchwork.kernel.org@lists.freedesktop.org On Mon, Mar 18, 2013 at 07:42:58AM +1000, Dave Airlie wrote: > On Mon, Mar 18, 2013 at 7:40 AM, Chris Wilson 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 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];