linux-kernel - [PATCH 1/2] blackfin: ptrace: fix the unsafe usage of mm/find_vma in is_user_addr

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20100527195609.GB25935@redhat.com>
Date:	Thu, 27 May 2010 21:56:09 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Mike Frysinger <vapier.adi@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	David Howells <dhowells@...hat.com>,
	Roland McGrath <roland@...hat.com>, linux-sh@...r.kernel.org,
	Paul Mundt <lethal@...ux-sh.org>,
	uclinux-dist-devel@...ckfin.uclinux.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH 1/2] blackfin: ptrace: fix the unsafe usage of mm/find_vma
	in is_user_addr_valid()

1. is_user_addr_valid() must not assume child->mm != NULL, we can
   race with SIGKILL. Use get_task_mm().

2. find_vma() needs mm->mmap_sem, we can race with another thread
   which shares ->mm with the tracee.

3. Move the simple FIXED_CODE_ checks up, before other potentially
   costly checks, and thus outside of mmap_sem.

4. Uninline it, it is fat and has 2 callers.

Note: we scan mm->context.sram_list under mmap_sem too. Currently this
is unnecessary, but see the next patch.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
---

 arch/blackfin/kernel/ptrace.c |   27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

--- 34-rc1/arch/blackfin/kernel/ptrace.c~IUAV_1_GET_MM_TAKE_SEM	2010-05-27 17:52:36.000000000 +0200
+++ 34-rc1/arch/blackfin/kernel/ptrace.c	2010-05-27 20:07:10.000000000 +0200
@@ -113,29 +113,40 @@ put_reg(struct task_struct *task, long r
 /*
  * check that an address falls within the bounds of the target process's memory mappings
  */
-static inline int is_user_addr_valid(struct task_struct *child,
+static int is_user_addr_valid(struct task_struct *child,
 				     unsigned long start, unsigned long len)
 {
+	struct mm_struct *mm;
 	struct vm_area_struct *vma;
 	struct sram_list_struct *sraml;
+	int ret = 0;
 
 	/* overflow */
 	if (start + len < start)
 		return -EIO;
+	if (start >= FIXED_CODE_START && start + len < FIXED_CODE_END)
+		return 0;
 
-	vma = find_vma(child->mm, start);
+	mm = get_task_mm(child);
+	if (!mm)
+		return -EIO;
+
+	down_read(&mm->mmap_sem);
+	vma = find_vma(mm, start);
 	if (vma && start >= vma->vm_start && start + len <= vma->vm_end)
-			return 0;
+			goto out;
 
-	for (sraml = child->mm->context.sram_list; sraml; sraml = sraml->next)
+	for (sraml = mm->context.sram_list; sraml; sraml = sraml->next)
 		if (start >= (unsigned long)sraml->addr
 		    && start + len < (unsigned long)sraml->addr + sraml->length)
-			return 0;
+			goto out;
 
-	if (start >= FIXED_CODE_START && start + len < FIXED_CODE_END)
-		return 0;
+	ret = -EIO;
+out:
+	up_read(&mm->mmap_sem);
+	mmput(mm);
 
-	return -EIO;
+	return ret;
 }
 
 /*

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/