gpu: host1x: Copy gathers before verification

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 <amerilainen@nvidia.com>
Signed-off-by: Terje Bergstrom <tbergstrom@nvidia.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>

1 files changed, 20 insertions, 31 deletions

diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c
index 5b9548f610f1..cc807667d8f1 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,20 +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);
-
-                if (!err)
-                        err = do_waitchks(job, host, g->bo);
+                        break;
 
+                err = do_waitchks(job, host, g->bo);
                 if (err)
                         break;
         }