linux-kernel - Re: Regression X Hangs at bootup -- PATCH

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090407221425.3dc1831e@schatten>
Date:	Tue, 7 Apr 2009 22:14:25 +0200
From:	Florian Mickler <florian@...kler.org>
To:	Eric Anholt <eric@...olt.net>
Cc:	LKML <linux-kernel@...r.kernel.org>, jbarnes@...tuousgeek.org,
	airlied@...ux.ie, keithp@...thp.com
Subject: Re: Regression X Hangs at bootup -- PATCH

On Tue, 07 Apr 2009 09:21:40 -0700
Eric Anholt <eric@...olt.net> wrote:


> drm_free's other arguments are unused memory debug leftovers.  I've
> got a patch I need to push at airlied to remove
> drm_malloc/drm_calloc/drm_free.
> 
in that case it is of course a non issue. but would you mind 
to add a note like 'this adds a memleak to i915_gem_put_relocs_to_user
which will be fixed in a followup patch', or just rebase it onto that
patch? 

anyhow, below patch is what i'm currently running.. if you rebase
the other version onto that patch of your's removing the
allocation thats fine too. if not, probably ok too. it's just that i
have too much time to care about this :)

sincerely,

Florian


p.s.: thx for doing this great work! 

From 868c938874d66aa3e507d8312cefc7730487f442 Mon Sep 17 00:00:00 2001
From: Florian Mickler <florian@...kler.org>
Date: Tue, 7 Apr 2009 18:41:32 +0200
Subject: [PATCH] Fix: use of uninitialized var

i915_gem_put_relocs_to_user returned an uninitialized value which
got returned to userspace. This caused libdrm in my setup to never
get out of a do{}while() loop retrying i915_gem_execbuffer.

result was hanging X, overheating of cpu and 2-3gb of logfile-spam.

This patch adresses the issue by
 1. initializing vars in this file where necessary
 2. correcting wrongly interpreted return values of
  copy_[from/to]_user

Nr. 2  helps libdrm from getting out of its loop and Nr. 1 helps
i915_gem_execbuffer to not think there was an error.

Signed-off-by: Florian Mickler <florian@...kler.org>
---
 drivers/gpu/drm/i915/i915_gem.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c index 1449b45..e73844a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -143,15 +143,18 @@ fast_shmem_read(struct page **pages,
 		int length)
 {
 	char __iomem *vaddr;
-	int ret;
+	int unwritten;
 
 	vaddr = kmap_atomic(pages[page_base >> PAGE_SHIFT], KM_USER0);
 	if (vaddr == NULL)
 		return -ENOMEM;
-	ret = __copy_to_user_inatomic(data, vaddr + page_offset,
length);
+	unwritten = __copy_to_user_inatomic(data, vaddr + page_offset,
length); kunmap_atomic(vaddr, KM_USER0);
 
-	return ret;
+	if (unwritten)
+		return -EFAULT;
+
+	return 0;
 }
 
 static inline int
@@ -3002,13 +3005,13 @@ i915_gem_get_relocs_from_user(struct
drm_i915_gem_exec_object *exec_list, drm_free(*relocs, reloc_count *
sizeof(**relocs), DRM_MEM_DRIVER);
 			*relocs = NULL;
-			return ret;
+			return -EFAULT;
 		}
 
 		reloc_index += exec_list[i].relocation_count;
 	}
 
-	return ret;
+	return 0;
 }
 
 static int
@@ -3017,7 +3020,7 @@ i915_gem_put_relocs_to_user(struct
drm_i915_gem_exec_object *exec_list, struct
drm_i915_gem_relocation_entry *relocs) {
 	uint32_t reloc_count = 0, i;
-	int ret;
+	int ret = 0;
 
 	for (i = 0; i < buffer_count; i++) {
 		struct drm_i915_gem_relocation_entry __user
*user_relocs; @@ -3036,6 +3039,9 @@ i915_gem_put_relocs_to_user(struct
drm_i915_gem_exec_object *exec_list, 
 	drm_free(relocs, reloc_count * sizeof(*relocs),
DRM_MEM_DRIVER); 
+	if (ret != 0)
+		ret = -EFAULT;
+
 	return ret;
 }
 
@@ -3308,10 +3314,12 @@ err:
 				   (uintptr_t) args->buffers_ptr,
 				   exec_list,
 				   sizeof(*exec_list) *
args->buffer_count);
-		if (ret)
+		if (ret) {
+			ret = -EFAULT;
 			DRM_ERROR("failed to copy %d exec entries "
 				  "back to user (%d)\n",
 				  args->buffer_count, ret);
+		}
 	}
 
 	/* Copy the updated relocations out regardless of current error
-- 
1.6.2


Download attachment "signature.asc" of type "application/pgp-signature" (198 bytes)