From patchwork Wed May 29 10:26:05 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arto Merilainen X-Patchwork-Id: 2628701 Return-Path: X-Original-To: patchwork-dri-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by patchwork2.kernel.org (Postfix) with ESMTP id 68FA2DF24C for ; Wed, 29 May 2013 10:31:15 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 950CDE63E7 for ; Wed, 29 May 2013 03:31:15 -0700 (PDT) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from hqemgate14.nvidia.com (hqemgate14.nvidia.com [216.228.121.143]) by gabe.freedesktop.org (Postfix) with ESMTP id E0EEFE623E for ; Wed, 29 May 2013 03:27:55 -0700 (PDT) Received: from hqnvupgp08.nvidia.com (Not Verified[216.228.121.13]) by hqemgate14.nvidia.com id ; Wed, 29 May 2013 03:27:53 -0700 Received: from hqemhub01.nvidia.com ([172.20.12.94]) by hqnvupgp08.nvidia.com (PGP Universal service); Wed, 29 May 2013 03:27:55 -0700 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Wed, 29 May 2013 03:27:55 -0700 Received: from amerilainen-lnx.Nvidia.com (172.20.144.16) by hqemhub01.nvidia.com (172.20.150.30) with Microsoft SMTP Server (TLS) id 8.3.298.1; Wed, 29 May 2013 03:27:54 -0700 From: Arto Merilainen To: , , , Subject: [PATCHv2 4/7] gpu: host1x: Copy gathers before verification Date: Wed, 29 May 2013 13:26:05 +0300 Message-ID: <1369823168-5396-5-git-send-email-amerilainen@nvidia.com> X-Mailer: git-send-email 1.8.1.5 In-Reply-To: <1369823168-5396-1-git-send-email-amerilainen@nvidia.com> References: <1369823168-5396-1-git-send-email-amerilainen@nvidia.com> X-NVConfidentiality: public MIME-Version: 1.0 Cc: tbergstrom@nvidia.com, linux-kernel@vger.kernel.org, Arto Merilainen 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 The firewall verified gather buffers before copying them. This allowed a malicious application to rewrite the buffer content by timing the rewrite carefully. This patch makes the buffer validation occur after copying the buffers. Signed-off-by: Arto Merilainen Signed-off-by: Terje Bergstrom --- drivers/gpu/host1x/job.c | 50 +++++++++++++++++++----------------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 028d2d4..cc80766 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -228,17 +228,15 @@ static unsigned int do_relocs(struct host1x_job *job, struct host1x_bo *cmdbuf) void *cmdbuf_page_addr = NULL; /* pin & patch the relocs for one gather */ - while (i < job->num_relocs) { + for (i = 0; i < job->num_relocs; i++) { struct host1x_reloc *reloc = &job->relocarray[i]; u32 reloc_addr = (job->reloc_addr_phys[i] + reloc->target_offset) >> reloc->shift; u32 *target; /* skip all other gathers */ - if (!(reloc->cmdbuf && cmdbuf == reloc->cmdbuf)) { - i++; + if (cmdbuf != reloc->cmdbuf) continue; - } if (last_page != reloc->cmdbuf_offset >> PAGE_SHIFT) { if (cmdbuf_page_addr) @@ -257,9 +255,6 @@ static unsigned int do_relocs(struct host1x_job *job, struct host1x_bo *cmdbuf) target = cmdbuf_page_addr + (reloc->cmdbuf_offset & ~PAGE_MASK); *target = reloc_addr; - - /* mark this gather as handled */ - reloc->cmdbuf = 0; } if (cmdbuf_page_addr) @@ -378,15 +373,13 @@ static int check_nonincr(struct host1x_firewall *fw) static int validate(struct host1x_firewall *fw, struct host1x_job_gather *g) { - u32 *cmdbuf_base; + u32 *cmdbuf_base = (u32 *)fw->job->gather_copy_mapped + + (g->offset / sizeof(u32)); int err = 0; if (!fw->job->is_addr_reg) return 0; - cmdbuf_base = host1x_bo_mmap(g->bo); - if (!cmdbuf_base) - return -ENOMEM; fw->words = g->words; fw->cmdbuf_id = g->bo; fw->offset = 0; @@ -453,10 +446,17 @@ out: static inline int copy_gathers(struct host1x_job *job, struct device *dev) { + struct host1x_firewall fw; size_t size = 0; size_t offset = 0; int i; + fw.job = job; + fw.dev = dev; + fw.reloc = job->relocarray; + fw.num_relocs = job->num_relocs; + fw.class = 0; + for (i = 0; i < job->num_gathers; i++) { struct host1x_job_gather *g = &job->gathers[i]; size += g->words * sizeof(u32); @@ -477,14 +477,19 @@ static inline int copy_gathers(struct host1x_job *job, struct device *dev) struct host1x_job_gather *g = &job->gathers[i]; void *gather; + /* Copy the gather */ gather = host1x_bo_mmap(g->bo); memcpy(job->gather_copy_mapped + offset, gather + g->offset, g->words * sizeof(u32)); host1x_bo_munmap(g->bo, gather); + /* Store the location in the buffer */ g->base = job->gather_copy; g->offset = offset; - g->bo = NULL; + + /* Validate the job */ + if (validate(&fw, g)) + return -EINVAL; offset += g->words * sizeof(u32); } @@ -497,15 +502,8 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) int err; unsigned int i, j; struct host1x *host = dev_get_drvdata(dev->parent); - struct host1x_firewall fw; DECLARE_BITMAP(waitchk_mask, host1x_syncpt_nb_pts(host)); - fw.job = job; - fw.dev = dev; - fw.reloc = job->relocarray; - fw.num_relocs = job->num_relocs; - fw.class = 0; - bitmap_zero(waitchk_mask, host1x_syncpt_nb_pts(host)); for (i = 0; i < job->num_waitchk; i++) { u32 syncpt_id = job->waitchk[i].syncpt_id; @@ -536,19 +534,11 @@ int host1x_job_pin(struct host1x_job *job, struct device *dev) if (job->gathers[j].bo == g->bo) job->gathers[j].handled = true; - err = 0; - - if (IS_ENABLED(CONFIG_TEGRA_HOST1X_FIREWALL)) - err = validate(&fw, g); - + err = do_relocs(job, g->bo); if (err) - dev_err(dev, "Job invalid (err=%d)\n", err); - - if (!err) - err = do_relocs(job, g->bo); + break; - if (!err) - err = do_waitchks(job, host, g->bo); + err = do_waitchks(job, host, g->bo); if (err) break; }