From patchwork Fri May 8 16:51:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rebecca N. Palmer" X-Patchwork-Id: 6366401 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 8AAC79F373 for ; Fri, 8 May 2015 16:53:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A280C20204 for ; Fri, 8 May 2015 16:52:59 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 9325720142 for ; Fri, 8 May 2015 16:52:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 70F756E97A; Fri, 8 May 2015 09:52:57 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from sender1.zohomail.com (sender1.zohomail.com [74.201.84.155]) by gabe.freedesktop.org (Postfix) with ESMTP id 807266E979 for ; Fri, 8 May 2015 09:52:56 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=zapps768; d=zoho.com; h=message-id:date:from:user-agent:mime-version:to:cc:subject:references:in-reply-to:content-type; b=W2P9oqxOYkRkQVxxIxffzjknMSLdt19o1ElmxLG1M5WIgvnPhNWzIsrIf4m3HF4fFWa+zklMCPRz 8g6VFbE2Q3Xi6yUDxvU7VEydh4kVf38DE1syr3E3d3qN0z9vvuB9 Received: from [192.168.1.102] (79-64-16-251.host.pobb.as13285.net [79.64.16.251]) by mx.zohomail.com with SMTPS id 1431103972281667.9482468800098; Fri, 8 May 2015 09:52:52 -0700 (PDT) Message-ID: <554CE99A.3030603@zoho.com> Date: Fri, 08 May 2015 17:51:38 +0100 From: "Rebecca N. Palmer" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.6.0 MIME-Version: 1.0 To: Daniel Vetter , Mika Kuoppala References: <554212BF.1040309@zoho.com> <20150508112448.GD15256@phenom.ffwll.local> <554CB99A.3090501@zoho.com> <87d22bgo73.fsf@gaia.fi.intel.com> <20150508142538.GG15256@phenom.ffwll.local> In-Reply-To: <20150508142538.GG15256@phenom.ffwll.local> Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH for 4.1] drm/i915: Don't clear exec_start if batch was not copied X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, FREEMAIL_FROM,RCVD_IN_DNSWL_MED,T_RP_MATCHES_RCVD,UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP i915_gem_execbuffer_parse returns the original batch_obj on batches it can't check (currently, chained batches). Don't clear offset or set I915_DISPATCH_SECURE in this case. Fixes 17cabf571e50677d980e9ab2a43c5f11213003ae. Signed-off-by: Rebecca Palmer --- > > This version also brings exec_start = 0 inside this check, as it > > appears to be there because the copying (i915_cmd_parser.c:1054) > > removes any offset the original might have had. > [pushed without comment on this] That makes this a bug in mainline as well, though I don't know of any actual problems it causes. (The security hole exists there too, but only with the declared-unsafe parameter i915.enable_ppgtt=2, hence the change of title) This fix was tested on 3e0283a (tip of Linus' tree), in the same way as before (libva tests, vlc video, glxgears, beignet tests). diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a3190e79..5ff8a64 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1548,33 +1548,39 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } if (i915_needs_cmd_parser(ring) && args->batch_len) { - batch_obj = i915_gem_execbuffer_parse(ring, + struct drm_i915_gem_object *parsed_batch_obj; + + parsed_batch_obj = i915_gem_execbuffer_parse(ring, &shadow_exec_entry, eb, batch_obj, args->batch_start_offset, args->batch_len, file->is_master); - if (IS_ERR(batch_obj)) { - ret = PTR_ERR(batch_obj); + if (IS_ERR(parsed_batch_obj)) { + ret = PTR_ERR(parsed_batch_obj); goto err; } - /* - * Set the DISPATCH_SECURE bit to remove the NON_SECURE - * bit from MI_BATCH_BUFFER_START commands issued in the - * dispatch_execbuffer implementations. We specifically - * don't want that set when the command parser is - * enabled. - * - * FIXME: with aliasing ppgtt, buffers that should only - * be in ggtt still end up in the aliasing ppgtt. remove - * this check when that is fixed. - */ - if (USES_FULL_PPGTT(dev)) - dispatch_flags |= I915_DISPATCH_SECURE; - - exec_start = 0; + if (parsed_batch_obj != batch_obj) { + /* + * Batch parsed and accepted: + * + * Set the DISPATCH_SECURE bit to remove the NON_SECURE + * bit from MI_BATCH_BUFFER_START commands issued in + * the dispatch_execbuffer implementations. We + * specifically don't want that set on batches the + * command parser has accepted. + * + * FIXME: with aliasing ppgtt, buffers that should only + * be in ggtt still end up in the aliasing ppgtt. + * remove USES_FULL_PPGTT check when that is fixed. + */ + if (USES_FULL_PPGTT(dev)) + dispatch_flags |= I915_DISPATCH_SECURE; + exec_start = 0; + batch_obj = parsed_batch_obj; + } } batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND;