linux-kernel - [PATCH] drm/panfrost: Avoid user size passed to kvmalloc()

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211216161603.983711-1-steven.price@arm.com>
Date:   Thu, 16 Dec 2021 16:16:03 +0000
From:   Steven Price <steven.price@....com>
To:     Daniel Vetter <daniel@...ll.ch>, David Airlie <airlied@...ux.ie>,
        Rob Herring <robh@...nel.org>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        Alyssa Rosenzweig <alyssa.rosenzweig@...labora.com>,
        Boris Brezillon <boris.brezillon@...labora.com>
Cc:     Steven Price <steven.price@....com>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: [PATCH] drm/panfrost: Avoid user size passed to kvmalloc()

panfrost_copy_in_sync() takes the number of fences from user space
(in_sync_count) and used to kvmalloc() an array to hold that number of
fences before processing them. This provides an easy method for user
space to trigger the OOM killer (by temporarily allocating large amounts
of kernel memory) or hit the WARN_ONCE() added by 7661809d493b ("mm:
don't allow oversized kvmalloc() calls").

Since we don't expect there to be a large number of fences we can
instead iterate over the fences one-by-one and avoid the temporary
allocation altogether. This also makes the code simpler.

Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Steven Price <steven.price@....com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 44 ++++++++-----------------
 1 file changed, 14 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 96bb5a465627..12ab55e5782c 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -186,47 +186,31 @@ panfrost_copy_in_sync(struct drm_device *dev,
 		  struct drm_panfrost_submit *args,
 		  struct panfrost_job *job)
 {
-	u32 *handles;
-	int ret = 0;
-	int i, in_fence_count;
-
-	in_fence_count = args->in_sync_count;
-
-	if (!in_fence_count)
-		return 0;
-
-	handles = kvmalloc_array(in_fence_count, sizeof(u32), GFP_KERNEL);
-	if (!handles) {
-		ret = -ENOMEM;
-		DRM_DEBUG("Failed to allocate incoming syncobj handles\n");
-		goto fail;
-	}
+	int i;
+	u32 __user *user_handles = u64_to_user_ptr(args->in_syncs);
 
-	if (copy_from_user(handles,
-			   (void __user *)(uintptr_t)args->in_syncs,
-			   in_fence_count * sizeof(u32))) {
-		ret = -EFAULT;
-		DRM_DEBUG("Failed to copy in syncobj handles\n");
-		goto fail;
-	}
-
-	for (i = 0; i < in_fence_count; i++) {
+	for (i = 0; i < args->in_sync_count; i++) {
 		struct dma_fence *fence;
+		u32 handle;
+		int ret;
+
+		ret = copy_from_user(&handle, user_handles + i,
+				     sizeof(handle));
+		if (ret)
+			return -EFAULT;
 
-		ret = drm_syncobj_find_fence(file_priv, handles[i], 0, 0,
+		ret = drm_syncobj_find_fence(file_priv, handle, 0, 0,
 					     &fence);
 		if (ret)
-			goto fail;
+			return ret;
 
 		ret = drm_sched_job_add_dependency(&job->base, fence);
 
 		if (ret)
-			goto fail;
+			return ret;
 	}
 
-fail:
-	kvfree(handles);
-	return ret;
+	return 0;
 }
 
 static int panfrost_ioctl_submit(struct drm_device *dev, void *data,
-- 
2.30.2