| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <20201023165136.561680-8-robdclark@gmail.com>
Date: Fri, 23 Oct 2020 09:51:08 -0700
From: Rob Clark <robdclark@...il.com>
To: dri-devel@...ts.freedesktop.org
Cc: linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org,
Rob Clark <robdclark@...omium.org>,
"Kristian H . Kristensen" <hoegsberg@...gle.com>,
Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel@...ll.ch>,
linux-kernel@...r.kernel.org (open list)
Subject: [PATCH v4 07/23] drm/msm/submit: Move copy_from_user ahead of locking bos
From: Rob Clark <robdclark@...omium.org>
We cannot switch to using obj->resv for locking without first moving all
the copy_from_user() ahead of submit_lock_objects(). Otherwise in the
mm fault path we aquire mm->mmap_sem before obj lock, but in the submit
path the order is reversed.
Signed-off-by: Rob Clark <robdclark@...omium.org>
Reviewed-by: Kristian H. Kristensen <hoegsberg@...gle.com>
---
drivers/gpu/drm/msm/msm_gem.h | 3 +
drivers/gpu/drm/msm/msm_gem_submit.c | 127 +++++++++++++++++----------
2 files changed, 82 insertions(+), 48 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 766ce278c74a..1f4e5d871a23 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -240,7 +240,10 @@ struct msm_gem_submit {
uint32_t type;
uint32_t size; /* in dwords */
uint64_t iova;
+ uint32_t offset;/* in dwords */
uint32_t idx; /* cmdstream buffer idx in bos[] */
+ uint32_t nr_relocs;
+ struct drm_msm_gem_submit_reloc *relocs;
} *cmd; /* array of size nr_cmds */
struct {
uint32_t flags;
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index aa5c60a7132d..b6c258c89290 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -62,11 +62,16 @@ static struct msm_gem_submit *submit_create(struct drm_device *dev,
void msm_gem_submit_free(struct msm_gem_submit *submit)
{
+ unsigned i;
+
dma_fence_put(submit->fence);
list_del(&submit->node);
put_pid(submit->pid);
msm_submitqueue_put(submit->queue);
+ for (i = 0; i < submit->nr_cmds; i++)
+ kfree(submit->cmd[i].relocs);
+
kfree(submit);
}
@@ -150,6 +155,66 @@ static int submit_lookup_objects(struct msm_gem_submit *submit,
return ret;
}
+static int submit_lookup_cmds(struct msm_gem_submit *submit,
+ struct drm_msm_gem_submit *args, struct drm_file *file)
+{
+ unsigned i, sz;
+ int ret = 0;
+
+ for (i = 0; i < args->nr_cmds; i++) {
+ struct drm_msm_gem_submit_cmd submit_cmd;
+ void __user *userptr =
+ u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));
+
+ ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd));
+ if (ret) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ /* validate input from userspace: */
+ switch (submit_cmd.type) {
+ case MSM_SUBMIT_CMD_BUF:
+ case MSM_SUBMIT_CMD_IB_TARGET_BUF:
+ case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
+ break;
+ default:
+ DRM_ERROR("invalid type: %08x\n", submit_cmd.type);
+ return -EINVAL;
+ }
+
+ if (submit_cmd.size % 4) {
+ DRM_ERROR("non-aligned cmdstream buffer size: %u\n",
+ submit_cmd.size);
+ ret = -EINVAL;
+ goto out;
+ }
+
+ submit->cmd[i].type = submit_cmd.type;
+ submit->cmd[i].size = submit_cmd.size / 4;
+ submit->cmd[i].offset = submit_cmd.submit_offset / 4;
+ submit->cmd[i].idx = submit_cmd.submit_idx;
+ submit->cmd[i].nr_relocs = submit_cmd.nr_relocs;
+
+ sz = array_size(submit_cmd.nr_relocs,
+ sizeof(struct drm_msm_gem_submit_reloc));
+ /* check for overflow: */
+ if (sz == SIZE_MAX) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ submit->cmd[i].relocs = kmalloc(sz, GFP_KERNEL);
+ ret = copy_from_user(submit->cmd[i].relocs, userptr, sz);
+ if (ret) {
+ ret = -EFAULT;
+ goto out;
+ }
+ }
+
+out:
+ return ret;
+}
+
static void submit_unlock_unpin_bo(struct msm_gem_submit *submit,
int i, bool backoff)
{
@@ -301,7 +366,7 @@ static int submit_bo(struct msm_gem_submit *submit, uint32_t idx,
/* process the reloc's and patch up the cmdstream as needed: */
static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *obj,
- uint32_t offset, uint32_t nr_relocs, uint64_t relocs)
+ uint32_t offset, uint32_t nr_relocs, struct drm_msm_gem_submit_reloc *relocs)
{
uint32_t i, last_offset = 0;
uint32_t *ptr;
@@ -327,18 +392,11 @@ static int submit_reloc(struct msm_gem_submit *submit, struct msm_gem_object *ob
}
for (i = 0; i < nr_relocs; i++) {
- struct drm_msm_gem_submit_reloc submit_reloc;
- void __user *userptr =
- u64_to_user_ptr(relocs + (i * sizeof(submit_reloc)));
+ struct drm_msm_gem_submit_reloc submit_reloc = relocs[i];
uint32_t off;
uint64_t iova;
bool valid;
- if (copy_from_user(&submit_reloc, userptr, sizeof(submit_reloc))) {
- ret = -EFAULT;
- goto out;
- }
-
if (submit_reloc.submit_offset % 4) {
DRM_ERROR("non-aligned reloc offset: %u\n",
submit_reloc.submit_offset);
@@ -694,6 +752,10 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
if (ret)
goto out;
+ ret = submit_lookup_cmds(submit, args, file);
+ if (ret)
+ goto out;
+
/* copy_*_user while holding a ww ticket upsets lockdep */
ww_acquire_init(&submit->ticket, &reservation_ww_class);
has_ww_ticket = true;
@@ -710,60 +772,29 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
goto out;
for (i = 0; i < args->nr_cmds; i++) {
- struct drm_msm_gem_submit_cmd submit_cmd;
- void __user *userptr =
- u64_to_user_ptr(args->cmds + (i * sizeof(submit_cmd)));
struct msm_gem_object *msm_obj;
uint64_t iova;
- ret = copy_from_user(&submit_cmd, userptr, sizeof(submit_cmd));
- if (ret) {
- ret = -EFAULT;
- goto out;
- }
-
- /* validate input from userspace: */
- switch (submit_cmd.type) {
- case MSM_SUBMIT_CMD_BUF:
- case MSM_SUBMIT_CMD_IB_TARGET_BUF:
- case MSM_SUBMIT_CMD_CTX_RESTORE_BUF:
- break;
- default:
- DRM_ERROR("invalid type: %08x\n", submit_cmd.type);
- ret = -EINVAL;
- goto out;
- }
-
- ret = submit_bo(submit, submit_cmd.submit_idx,
+ ret = submit_bo(submit, submit->cmd[i].idx,
&msm_obj, &iova, NULL);
if (ret)
goto out;
- if (submit_cmd.size % 4) {
- DRM_ERROR("non-aligned cmdstream buffer size: %u\n",
- submit_cmd.size);
+ if (!submit->cmd[i].size ||
+ ((submit->cmd[i].size + submit->cmd[i].offset) >
+ msm_obj->base.size / 4)) {
+ DRM_ERROR("invalid cmdstream size: %u\n", submit->cmd[i].size * 4);
ret = -EINVAL;
goto out;
}
- if (!submit_cmd.size ||
- ((submit_cmd.size + submit_cmd.submit_offset) >
- msm_obj->base.size)) {
- DRM_ERROR("invalid cmdstream size: %u\n", submit_cmd.size);
- ret = -EINVAL;
- goto out;
- }
-
- submit->cmd[i].type = submit_cmd.type;
- submit->cmd[i].size = submit_cmd.size / 4;
- submit->cmd[i].iova = iova + submit_cmd.submit_offset;
- submit->cmd[i].idx = submit_cmd.submit_idx;
+ submit->cmd[i].iova = iova + (submit->cmd[i].offset * 4);
if (submit->valid)
continue;
- ret = submit_reloc(submit, msm_obj, submit_cmd.submit_offset,
- submit_cmd.nr_relocs, submit_cmd.relocs);
+ ret = submit_reloc(submit, msm_obj, submit->cmd[i].offset * 4,
+ submit->cmd[i].nr_relocs, submit->cmd[i].relocs);
if (ret)
goto out;
}
--
2.26.2
Powered by blists - more mailing lists