From patchwork Fri Oct 2 13:16:53 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michel Thierry X-Patchwork-Id: 7315441 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 949369F1D5 for ; Fri, 2 Oct 2015 13:17:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id B0AAE208C4 for ; Fri, 2 Oct 2015 13:16:59 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 86794208C2 for ; Fri, 2 Oct 2015 13:16:58 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E0ECB6E168; Fri, 2 Oct 2015 06:16:57 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 7C1896E168 for ; Fri, 2 Oct 2015 06:16:56 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP; 02 Oct 2015 06:16:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,623,1437462000"; d="scan'208";a="802166618" Received: from michelth-linux2.isw.intel.com ([10.102.226.189]) by fmsmga001.fm.intel.com with ESMTP; 02 Oct 2015 06:16:54 -0700 From: Michel Thierry To: intel-gfx@lists.freedesktop.org Date: Fri, 2 Oct 2015 14:16:53 +0100 Message-Id: <1443791813-30551-1-git-send-email-michel.thierry@intel.com> X-Mailer: git-send-email 2.6.0 In-Reply-To: <1443715175-32567-1-git-send-email-michel.thierry@intel.com> References: <1443715175-32567-1-git-send-email-michel.thierry@intel.com> Subject: [Intel-gfx] [PATCH v2] drm/i915: prevent out of range pt in the PDE macros (take 3) 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: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable 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 We tried to fix this in commit fdc454c1484a ("drm/i915: Prevent out of range pt in gen6_for_each_pde"). But the static analyzer still complains that, just before we break due to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an iter value that is bigger than I915_PDES. Of course, this isn't really a problem since no one uses pt outside the macro. Still, every single new usage of the macro will create a new issue for us to mark as a false positive. Also, Paulo re-started the discussion a while ago [1], but didn't end up implemented. In order to "solve" this "problem", this patch takes the ideas from Chris and Dave, but that check would change the desired behavior of the code, because the object (for example pdp->page_directory[iter]) can be null during init/alloc, and C would take this as false, breaking the for loop immediately. This has been already verified with "static analysis tools". [1]http://lists.freedesktop.org/archives/intel-gfx/2015-June/068548.html v2: Make it a single statement, while preventing the common subexpression elimination (Chris) Cc: Paulo Zanoni Cc: Chris Wilson Cc: Dave Gordon Signed-off-by: Michel Thierry Reviewed-by: Chris Wilson Reviewed-by: Dave Gordon --- drivers/gpu/drm/i915/i915_gem_gtt.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h index 9fbb07d..a216397 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.h +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h @@ -394,7 +394,8 @@ struct i915_hw_ppgtt { */ #define gen6_for_each_pde(pt, pd, start, length, temp, iter) \ for (iter = gen6_pde_index(start); \ - pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \ + length > 0 && iter < I915_PDES ? \ + (pt = (pd)->page_table[iter]), 1 : 0; \ iter++, \ temp = ALIGN(start+1, 1 << GEN6_PDE_SHIFT) - start, \ temp = min_t(unsigned, temp, length), \ @@ -459,7 +460,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr) */ #define gen8_for_each_pde(pt, pd, start, length, temp, iter) \ for (iter = gen8_pde_index(start); \ - pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \ + length > 0 && iter < I915_PDES ? \ + (pt = (pd)->page_table[iter]), 1 : 0; \ iter++, \ temp = ALIGN(start+1, 1 << GEN8_PDE_SHIFT) - start, \ temp = min(temp, length), \ @@ -467,8 +469,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr) #define gen8_for_each_pdpe(pd, pdp, start, length, temp, iter) \ for (iter = gen8_pdpe_index(start); \ - pd = (pdp)->page_directory[iter], \ - length > 0 && (iter < I915_PDPES_PER_PDP(dev)); \ + length > 0 && (iter < I915_PDPES_PER_PDP(dev)) ? \ + (pd = (pdp)->page_directory[iter]), 1 : 0; \ iter++, \ temp = ALIGN(start+1, 1 << GEN8_PDPE_SHIFT) - start, \ temp = min(temp, length), \ @@ -476,8 +478,8 @@ static inline uint32_t gen6_pde_index(uint32_t addr) #define gen8_for_each_pml4e(pdp, pml4, start, length, temp, iter) \ for (iter = gen8_pml4e_index(start); \ - pdp = (pml4)->pdps[iter], \ - length > 0 && iter < GEN8_PML4ES_PER_PML4; \ + length > 0 && iter < GEN8_PML4ES_PER_PML4 ? \ + (pdp = (pml4)->pdps[iter]), 1 : 0; \ iter++, \ temp = ALIGN(start+1, 1ULL << GEN8_PML4E_SHIFT) - start, \ temp = min(temp, length), \